Skip to content

Commit cff091f

Browse files
ezyangezyang
committed
Make Pull-Request-resolved hyphenated to be valid trailer
Previously, we used the syntax "Pull Request resolved", however, this is not a valid Git trailer as there are spaces in the key. Replacing the spaces with hyphens fixes this, while not impeding GitHub's auto-resolve capabilities (as it only matches for resolved at the tail of the string). I also use git interpret-trailers to handle trailer modification, which makes ghstack more robust when there are pre-existing trailers in the commit message (as all trailers must be together in a blog, there must not be any double newlines.) Signed-off-by: Edward Z. Yang <[email protected]> ghstack-source-id: dfa949d ghstack-comment-id: 2726090949 Pull-Request-resolved: ezyang#278 ghstack-source-id: fe9507c ghstack-comment-id: 2726097428 Pull Request resolved: ezyang#279
1 parent f19adfb commit cff091f

13 files changed

+48
-47
lines changed

‎src/ghstack/diff.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515

1616
RAW_PULL_REQUEST_RESOLVED = (
17-
r"Pull Request resolved: "
17+
r"(Pull Request resolved|Pull-Request-resolved): "
1818
r"https://{github_url}/(?P<owner>[^/]+)/(?P<repo>[^/]+)/pull/(?P<number>[0-9]+)"
1919
)
2020

@@ -93,7 +93,7 @@ class Diff:
9393
# be the same.
9494
source_id: str
9595

96-
# The contents of 'Pull Request resolved'. This is None for
96+
# The contents of 'Pull-Request-resolved'. This is None for
9797
# diffs that haven't been submitted by ghstack. For BC reasons,
9898
# this also accepts gh-metadata.
9999
pull_request_resolved: Optional[PullRequestResolved]

‎src/ghstack/submit.py

+16-13
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ def elaborate_diff(
815815
convert a pre-existing PR in the old style to the new format which
816816
supports bidirectional syncing. If you would like to blow away the old
817817
PR and start anew, edit the Summary in the Phabricator diff to delete
818-
the line 'Pull Request resolved' and then run ghexport again.
818+
the line 'Pull-Request-resolved' and then run ghexport again.
819819
"""
820820
)
821821

@@ -832,7 +832,7 @@ def elaborate_diff(
832832
to export your diff.
833833
834834
If you think this is in error, edit the Summary in the Phabricator diff
835-
to delete the line 'Pull Request resolved' and then run ghexport again.
835+
to delete the line 'Pull-Request-resolved' and then run ghexport again.
836836
"""
837837
)
838838
else:
@@ -963,17 +963,20 @@ def process_commit(
963963
commit_msg = self._update_source_id(diff.summary, elab_diff)
964964
else:
965965
# Need to insert metadata for the first time
966-
commit_msg = "".join(
967-
[
968-
f"{strip_mentions(diff.summary.rstrip())}\n\n",
969-
f"ghstack-source-id: {diff.source_id}\n",
970-
(
971-
f"ghstack-comment-id: {elab_diff.comment_id}\n"
972-
if self.direct
973-
else ""
974-
),
975-
f"Pull Request resolved: {pull_request_resolved.url()}",
976-
]
966+
# TODO: Probably quicker if we reimplement reinterpret-trailers
967+
# in Python
968+
commit_msg = self.sh.git(
969+
"interpret-trailers",
970+
"--trailer",
971+
f"ghstack-source-id: {diff.source_id}",
972+
*(
973+
["--trailer", f"ghstack-comment-id: {elab_diff.comment_id}"]
974+
if self.direct
975+
else []
976+
),
977+
"--trailer",
978+
f"Pull-Request-resolved: {pull_request_resolved.url()}",
979+
input=strip_mentions(diff.summary.rstrip()),
977980
)
978981

979982
return DiffMeta(

‎test/land/default_branch_change.py.test

+8-8
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ assert_expected_inline(
4848
assert_expected_inline(
4949
get_upstream_sh().git("log", "--oneline", "main"),
5050
"""\
51-
8b023bd Commit A
51+
6f717c4 Commit A
5252
dc8bfe4 Initial commit""",
5353
)
5454

@@ -82,15 +82,15 @@ assert_github_state(
8282

8383
This is commit B
8484

85-
* cfc0530 Initial 2
85+
* 53a3584 Initial 2
8686

8787
Repository state:
8888

89-
* cfc0530 (gh/ezyang/2/head)
89+
* 53a3584 (gh/ezyang/2/head)
9090
| Initial 2
91-
* bf457db (gh/ezyang/2/base)
91+
* c55ff15 (gh/ezyang/2/base)
9292
| Initial 2 (base update)
93-
* 8b023bd (main, gh/ezyang/1/orig)
93+
* 6f717c4 (main, gh/ezyang/1/orig)
9494
| Commit A
9595
| * 36fcfdf (gh/ezyang/1/head)
9696
| | Initial 1
@@ -107,13 +107,13 @@ gh_land(diff2.pr_url)
107107
assert_expected_inline(
108108
get_upstream_sh().git("log", "--oneline", "master"),
109109
"""\
110-
a677f4e Commit B
111-
8f51278 Commit A
110+
35747c0 Commit B
111+
b2ec056 Commit A
112112
dc8bfe4 Initial commit""",
113113
)
114114
assert_expected_inline(
115115
get_upstream_sh().git("log", "--oneline", "main"),
116116
"""\
117-
8b023bd Commit A
117+
6f717c4 Commit A
118118
dc8bfe4 Initial commit""",
119119
)

‎test/land/ff.py.test

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ gh_land(pr_url)
1111
assert_expected_inline(
1212
get_upstream_sh().git("log", "--oneline", "master"),
1313
"""\
14-
8b023bd Commit A
14+
6f717c4 Commit A
1515
dc8bfe4 Initial commit""",
1616
)
1717

‎test/land/ff_stack.py.test

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ gh_land(pr_url)
1616
assert_expected_inline(
1717
get_upstream_sh().git("log", "--oneline", "master"),
1818
"""\
19-
b2d9b84 Commit B
20-
fe65c88 Commit A
19+
2f2f3f6 Commit B
20+
3db0d3b Commit A
2121
dc8bfe4 Initial commit""",
2222
)

‎test/land/ff_stack_two_phase.py.test

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ gh_land(pr_url2)
1818
assert_expected_inline(
1919
get_upstream_sh().git("log", "--oneline", "master"),
2020
"""\
21-
b2d9b84 Commit B
22-
fe65c88 Commit A
21+
2f2f3f6 Commit B
22+
3db0d3b Commit A
2323
dc8bfe4 Initial commit""",
2424
)

‎test/land/invalid_resubmit.py.test

+5-5
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pr_url = diff.pr_url
1111
gh_land(pr_url)
1212

1313
write_file_and_add("file2.txt", "A")
14-
git("commit", "--amend")
14+
git("commit", "--amend", "--no-edit")
1515
assert_expected_raises_inline(
1616
RuntimeError,
1717
lambda: gh_submit("Update"),
@@ -44,15 +44,15 @@ else:
4444

4545
This is commit A
4646

47-
* edc8acf New PR
47+
* f014df7 New PR
4848

4949
Repository state:
5050

51-
* edc8acf (gh/ezyang/1/head)
51+
* f014df7 (gh/ezyang/1/head)
5252
| New PR
53-
* b38da4f (gh/ezyang/1/base)
53+
* 69e4b60 (gh/ezyang/1/base)
5454
| New PR (base update)
55-
* 8b023bd (HEAD -> master)
55+
* 6f717c4 (HEAD -> master)
5656
| Commit A
5757
* dc8bfe4
5858
Initial commit

‎test/land/non_ff.py.test

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ gh_land(pr_url)
1717
assert_expected_inline(
1818
get_upstream_sh().git("log", "--oneline", "master"),
1919
"""\
20-
a3996ab Commit A
20+
b327b28 Commit A
2121
38808c0 Commit U
2222
dc8bfe4 Initial commit""",
2323
)

‎test/land/non_ff_stack_two_phase.py.test

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ gh_land(pr_url2)
2222
assert_expected_inline(
2323
get_upstream_sh().git("log", "--oneline", "master"),
2424
"""\
25-
643692e Commit B
26-
3a731f4 Commit A
25+
c86afe5 Commit B
26+
30e22bc Commit A
2727
a8ca27f Commit C
2828
dc8bfe4 Initial commit""",
2929
)

‎test/land/update_after_land.py.test

+4-4
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,16 @@ else:
5555

5656
This is commit B
5757

58-
* 2ffb7a7 Run 3
58+
* 7d34855 Run 3
5959
* 16e1e12 Initial 1
6060

6161
Repository state:
6262

63-
* 2ffb7a7 (gh/ezyang/2/head)
63+
* 7d34855 (gh/ezyang/2/head)
6464
|\\ Run 3
65-
| * 8ff40f5 (gh/ezyang/2/base)
65+
| * 9a04232 (gh/ezyang/2/base)
6666
| |\\ Run 3 (base update)
67-
| | * d6454e4 (HEAD -> master)
67+
| | * 16f0d55 (HEAD -> master)
6868
| | | Commit A
6969
| | * 7f0288c
7070
| | | Commit U

‎test/submit/do_not_revert_local_commit_msg_on_skip.py.test

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ if is_direct():
1717

1818
ghstack-source-id: ac00f28640afe01e4299441bb5041cdf06d0b6b4
1919
ghstack-comment-id: 1500
20-
Pull Request resolved: https://github.com/pytorch/pytorch/pull/500""",
20+
Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""",
2121
)
2222
else:
2323
assert_expected_inline(
@@ -28,7 +28,7 @@ else:
2828
This is commit ARGLE
2929

3030
ghstack-source-id: ac00f28640afe01e4299441bb5041cdf06d0b6b4
31-
Pull Request resolved: https://github.com/pytorch/pytorch/pull/500""",
31+
Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""",
3232
)
3333

3434
if is_direct():

‎test/submit/strip_mentions.py.test

+2-4
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,9 @@ if is_direct():
4444
This is my first commit, hello foobar Ivan
4545

4646
Signed-off-by: [email protected]
47-
4847
ghstack-source-id: ddf506901b8d3c2b4079ec39e564f19bd5468397
4948
ghstack-comment-id: 1500
50-
Pull Request resolved: https://github.com/pytorch/pytorch/pull/500""",
49+
Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""",
5150
)
5251
else:
5352
assert_expected_inline(
@@ -60,7 +59,6 @@ else:
6059
This is my first commit, hello foobar Ivan
6160

6261
Signed-off-by: [email protected]
63-
6462
ghstack-source-id: ddf506901b8d3c2b4079ec39e564f19bd5468397
65-
Pull Request resolved: https://github.com/pytorch/pytorch/pull/500""",
63+
Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""",
6664
)

‎uv.lock

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)