1826125 - Add a mozphab setting to change the default bookmark/branch name format for patch

Status: RESOLVED FIXED

(Conduit :: moz-phab, enhancement)

Description

[Shane Hughes [:aminomancer]]

Hi there! Lately I've been using the GitLens extension for VS Code in conjunction with a git clone of mozilla-central and it has been working great. It adds the ability to organize your branches into folders and subfolders by using the / token in your branch names. So for my own revisions I call them something like bugs/123456_bugDescription and then they all go in the bugs folder.

But in order to do the same with patches applied from Phabricator, I have to rename the branch manually after patching, since mozphab throws if I try to include / in the name of a branch with the --name argument. It would be nice to figure out what's going on in that case, maybe slash is an issue for some VCS but not all.

But ideally we could just omit the --name argument but have a setting in .moz-phab-config or somewhere, that basically defines the patch bookmark name format, like the current phab-D{revision_id}. If I replaced the hyphen - with a forward slash / that would put all of the patches in a phab folder. For that, I guess there would need to be a replacement token for the revision ID, like {revision_id} or even %s or something along those lines.

If it's easy, it'd also be cool to have a replacement token for the bugzilla bug ID (if the patch has one), so we could include that in the template and organize patches first by bug and then by revision ID.

Thanks!

Comment 1

[Connor Sheehan [:sheehan]]

(In reply to Shane Hughes [:aminomancer] from comment #0)

Hi there! Lately I've been using the GitLens extension for VS Code in conjunction with a git clone of mozilla-central and it has been working great. It adds the ability to organize your branches into folders and subfolders by using the / token in your branch names. So for my own revisions I call them something like bugs/123456_bugDescription and then they all go in the bugs folder.

But in order to do the same with patches applied from Phabricator, I have to rename the branch manually after patching, since mozphab throws if I try to include / in the name of a branch with the --name argument. It would be nice to figure out what's going on in that case, maybe slash is an issue for some VCS but not all.

I'm not seeing this problem. Try wrapping the value of the --name argument in quotes from the CLI:

❯ moz-phab patch D173445 --apply-to here --name "blah/blah"
Patching revisions: D173444 D173445
Created branch blah/blah
[blah/blah 34e78c4] uplift: add periods to the end of comments (Bug 1818383) r?dkl,zeid
 Date: Thu Mar 23 10:49:35 2023 -0400
 1 file changed, 2 insertions(+), 2 deletions(-)
D173444 applied
[blah/blah afe730e] uplift: add a link to Release Management Wiki to `moz-phab uplift --help` (Bug 1818383) r?dkl,zeid
 Date: Thu Mar 23 11:12:49 2023 -0400
 1 file changed, 6 insertions(+), 1 deletion(-)
D173445 applied
❯ git show
commit afe730e0e822fd33f99d10ef13a699fceeadf4d6 (HEAD -> blah/blah)
Author: Connor Sheehan <[e-mail]>
Date:   Thu Mar 23 11:12:49 2023 -0400
...

If that doesn't work (maybe some weirdness with the Windows terminal?), please paste the full error you're seeing. If you could run with DEBUG=1 in the environment that would be great too.

But ideally we could just omit the --name argument but have a setting in .moz-phab-config or somewhere, that basically defines the patch bookmark name format, like the current phab-D{revision_id}. If I replaced the hyphen - with a forward slash / that would put all of the patches in a phab folder. For that, I guess there would need to be a replacement token for the revision ID, like {revision_id} or even %s or something along those lines.

If it's easy, it'd also be cool to have a replacement token for the bugzilla bug ID (if the patch has one), so we could include that in the template and organize patches first by bug and then by revision ID.

Thanks!

This is an interesting idea! Making the bookmark name format configurable would be very straightforward. Adding the Bugzilla bug ID would be slightly more work. I'll take a crack at the former when I have some spare cycles this week.

Comment 2

[Shane Hughes [:aminomancer]]

(In reply to Connor Sheehan [:sheehan] from comment #1)

I'm not seeing this problem. Try wrapping the value of the --name argument in quotes from the CLI:
If that doesn't work (maybe some weirdness with the Windows terminal?), please paste the full error you're seeing. If you could run with DEBUG=1 in the environment that would be great too.

Ah, yeah I'm using Windows terminal. Though I also ran into this in a MozillaBuild shell integrated terminal I set up in VS Code. I used double quotes the first time, though I have seen them behave differently with fzf depending on the terminal. I just tried it in the MozillaBuild shell in a regular cmd.exe prompt, and it was successful that time. So, sounds like that was just an issue with my environment and not a big deal.

This is an interesting idea! Making the bookmark name format configurable would be very straightforward. Adding the Bugzilla bug ID would be slightly more work. I'll take a crack at the former when I have some spare cycles this week.

Awesome, thanks for taking the time! Including the phab revision ID alone is super helpful.

Comment 3

[Connor Sheehan [:sheehan]]

Attachment: patch: make branch name configurable (Bug 1826125) r?zeid

Adds a patch.branch_name_template config knob to control
the name used as the branch name for a new bookmark/branch.
This knob takes a single format template, rev_id, which
is the rev id of the revision being patched. The current
default value is phab-D{rev_id} and that is set as the
default value in the config. Move branch name resolution
into a separate resolve_branch_name function and add a
test for it.