thanks @ton - these look very actionable:

• The template is actually generated server side, not in arcanist, so it just a matter of getting the BSD Phorge server to have the right opinions.
• Some of the sections can be configured already
• We can add hooks for the rest (both line breaks and other template parts), so that a local extension will be able to modify it
• BSD will be able to write the extension with their own rules.

Figuring out what already exists and where to add the required extensions will take some research, but that's totally possible.

Alternatively - there's also arc land --hold, which does almost everything except for the git push, which would allow the user to update the commit message. Have anyone tried that?

re @avivey : arc land --hold sort of works but look at the ergonomics:

1. User runs arc patch Dxxxx to download the revision
2. User runs arc land --hold to create a commit but not push it
3. User runs git commit --amend to fix the commit message
4. User runs git push

Why doing that when user can just?

1. Download the .patch with curl
2. Pipe it into git am --interactive

There is probably something I am not understanding, but users don't either.
The value-add of using arcanist is far from being obvious to the user.

The template is actually generated server side, not in arcanist, so it just a matter of getting the BSD Phorge server to have the right opinions.
Some of the sections can be configured already

Please point me in the right place in the code where I can find the template

In T15364#8281, @ton wrote:

The template is actually generated server side, not in arcanist, so it just a matter of getting the BSD Phorge server to have the right opinions.
Some of the sections can be configured already

Please point me in the right place in the code where I can find the template

It's a bit more complicated then that. IIRC, it's composed of individual Field objects, that combine their powers to form the message.

Let's see if I can find all the parts.

You can add Custom Fields using this guide: https://we.phorge.it/book/phorge/article/custom_fields/
and there's some configuration at /config/edit/differential.fields/ (though I don't see "subscribers" there).

DifferentialGetCommitMessageConduitAPIMethod is the conduit method that returns the message, so that's the place to start. It basically looks over all the fields, and asks them to renderFieldValue(), and uses that. I think that's because the output of this method (commit message) is the input to the code that parses the "arc diff" submit form.

That makes me think that maybe this isn't as easy as I was thinking - in this use-case, we want the commit message "for landing" to be different from the commit message "to keep working".

So some of it could be changed - e.g., DifferentialSummaryCommitMessageField could have renderFieldValue() that does wrapping (there's code for wrapping somewhere, I think), and some fields can be removed from both the commit message and from the arc-diff form, which would be OK sometimes. But hiding some fields for the commit message "for landing" is much harder.

Looks like there's an open task https://secure.phabricator.com/T2920 about this - with the last comment being basically "you should make local changes to suite your project".

1. User runs arc patch Dxxxx to download the revision
2. User runs arc land --hold to create a commit but not push it
3. User runs git commit --amend to fix the commit message
4. User runs git push

I think arc land in this flow is not actually doing anything useful - the message is already obtained by arc patch.

Also, the "best theoretical solution" might be some home-brew "land from GUI" button (https://secure.phabricator.com/T182) - I've implemented that a couple of times at least.

We can probably come up with a way to allow an extension to customize the "render template" implementation without forking (e.g., just add a hook in differential.getcommitmessage), but it would have to be very carful about the whole form-thing.
Maybe just adding an "for landing" flag in differential.getcommitmessage (or adding a different method) would solve this problem...

In an ideal world the solution should be as simple as having a template somewhere in {{repo_root}}/.arcanist/commit_template.j2 that arcanist injects values in and renders.

I am using python's Jinja2 for illustration purposes

• https://palletsprojects.com/p/jinja/
• https://jinja.palletsprojects.com/en/3.1.x/templates/

What are the reasons for doing commit message rendering on the server side?

What are the reasons for doing commit message rendering on the server side?

It's much easier to control the server side then the client side.

• Some fields might be too complex to render properly using a template engine (and also, PHP started life as a template engine, and look where that got us...)
• Calculating some fields may involve data that shouldn't be available to users, although I can't come up with an example.
• In multi-repo organizations, you'll have to duplicate/sync the template across repos.
• "Opinionated" is something we like - it's easier to code the system's opinions in code and let interested parties fork (customize the system's opinion) than it is to make a very customizable system and properly support things like "user provided templates".

but the technical reason is that the commit generated is tightly coupled with the Revision Creation Form, and that has to defined server-side as well.

FWIW I think the test plan is configurable (that requirement can be disabled in phabricator's config server side which should remove it from commit messages in arc)

Ok, so here's my thoughts on moving this forward:

1. Create new conduit method, differential.getcommitmessageforlanding (or something shorter).
2. Make arc land use this method if it exists, fallback to differential.getcommitmessage. Also add a flag to arc amend to try this method - but probably not for arc patch.
3. The new method will have basically the same logic as the differential.getcommitmessage, but will allow extensions to customize each field, as well as the whole message.
4. FreeBSD will write the extension (with our help) to make the commit message match their commit guidelines.

Yea, today I have about 50 lines of shell scripting that gets the different bits of data from the condon API point and then bashes them into a nearly acceptable commit message....

It would be nice, imho, if arc patch could do the right commit formatting too so I could do that and test it locally and have good commit messages to review as part of that process (you never really know from GUIs what you'll get)... That would also let me pull the dependent patches... while arc land works only on the given refs, and their ancestors, it is terrible for the following work flow:

Some user Joe uploads 20 patches (with our git arc tool, so they have proper child relationships).
arc patch D1234 (the last of these) will snag all 20 patches and put them into a branch (either current one, or one it creates).
I test these. At this point, I'm good to go if the commit messages are good and have the extra metadata details that we like to include. I could just git push the results. Except there's no metadata about who reviewed it in the commit messages yet (it would be ideal if there were a step to refresh this, and maybe arc land would be that step).

It would also be nice to override the author since phabricator, at least, has this bad habit of making actual user names kinda tricky to get. If the automation picked the wrong thing for whatever reason (including pilot error on the part of the original submitter) that the author could be overridden. Not required because git rebase --exec 'git commit --amend --author=X' could be done...

Anyway, that's my take at the moment... I'll be the first to admit I'm early in my learning curve so my perspective is colored by my most recent climbing activities :)

mmm... I've never really worked where many changes are made of lots of dependent revisions - it's possible I've never even seen a chain of 3 revisions in the same repo. So I'm not sure about workflows for this kind of scenarios.

Are these kind of chains common? I wouldn't be surprised to learn that arc tooling in general isn't really good for it.

Chaining Diffs is something that we would like to use more often, and it is definitely not used enough today. FreeBSD is a large codebase and making any change that is bigger than trivial often involves touching multiple parts of the system, the most difficult kind of change is architectural changes. We are limited by our tooling to a certain degree:

• Keeping everything in one Diff makes it very difficult to review as well as reach consensus, as there are often multiple responsible parties involved. It also violates the ethos of Phab/Phorge of having small atomic changes.
• Breaking changes down in smaller chunks has lots of downsides too - it is very hard to track and changes often get lost in depths of Differential.

The burden is often on the reviewer to tie all the loose ends together, and much time is spent on annoying things like metadata, attribution, commit messages etc.
It is also difficult for users to use chaining. AFAIK this functionality is not available via arcanist.

That is to say that chains of 20 diffs are not super common today, but mostly due to the reasons above rather than anything else.. We would like use it more.

"tooling for chains of diffs" might need its own topic, with a design of the what the whole thing should look like in an ideal world, and how to get to it.
Even if not all of the stuff fits nicely into Phorge, there's probably a lot that can work, and some conduit methods can be added to implement the rest.

In T15364#8952, @avivey wrote:

"tooling for chains of diffs" might need its own topic, with a design of the what the whole thing should look like in an ideal world, and how to get to it.
Even if not all of the stuff fits nicely into Phorge, there's probably a lot that can work, and some conduit methods can be added to implement the rest.

I briefly summarized the problem and some prior art over at T15410