Page MenuHomePhabricator

Allow to configure arc commit message
Closed, DuplicatePublic

Description

See also: http://stackoverflow.com/questions/34679013/is-possible-to-modify-arcanist-differential-template

I would like to request the ability to change the contents of the commit message. I.e. until we finally get T5000, at least give people an option to prevent arc from messing with commit messages and adding useless noise. I see how the stuff you add there can be useful for some projects, but it's not useful for many others.

The way I see this being implemented is by using some sort of dot file to set the arc commit template, similar to the git commit template. This file should be tracked in the VCS, akin to .arcconfig.

Alternatively, offer a way to configure arc diff default arguments via .arcconfig, such that one can always use --verbatim and the normal git commit template (cf. https://secure.phabricator.com/w/guides/arcanist_workflows/). Having to tell new people to --verbatim when using arc, or creating custom aliases, is simply bad.

Event Timeline

What specific modifications are you interested in making to the messages?

Also, please work with other KDE project members to discuss KDE priorities and needs downstream, then have a single point of contact (nominally, @bcooksley) bring them upstream once there is consensus on the nature and priority of problems. Currently, you, @abika, and @bcooksley are all independently interacting with this upstream.

When multiple individuals from a single project talk directly to us instead of talking with each other, it tends to create confusion and miscommunication which requires extra work for us to resolve. For example, T12255 duplicated the existing downstream task but omitted important information. We are a small team (two people) and time we spend spend deduplicating tasks, asking for context, and making sure everyone is on the same page is time we aren't spending actually implementing features or solving problems.

Beyond communication overhead, receiving requests from multiple points of contact prevents us from identifying which problems are most important to KDE. In particular, we don't currently have enough information to assess how important this and T12256/T12257 are relative to one another for KDE, and can only know this if the project has a single point of contact who keeps track of everything and can identify priorities for the project.

Context is also tremendously important to this upstream. We will almost never implement a feature without a deep understanding of why the feature is being requested and the context surrounding the request. (For details, see Describing Root Problems.) One reason that this is important is that we need to know how to change a feature when the product changes.

Also note that the barrier for features which would see use by only one install is extremely high: you are essentially asking us to develop and maintain a feature for only you, indefinitely. You should approach feature requests with modest expectations if you are ultimately trying to solve problems which are only experienced by KDE, or only experienced by KDE and a small collection of similar projects but not by software development organizations in general.

Regarding what modifications I would like to see:

See e.g.: https://phabricator.kde.org/R32:8a170619bc0cb5c6cfbe06383cb9973f31adfdb9

The commit message is:

Feature: Add file template for QObject with pimpl

Summary:
The current QObject template has all private members exposed
in the class, and there are reasons to use a pimpl variant
instead.
As the current file template logic does not allow to control
the number of files generated (like only generating the private
header if the user toggles a config option), the QObject pimpl
template is a complete separate template.

Both QObject templates share template snippets where useful.

Reviewers: KDevelop, kfunk, mwolff

Reviewed By: KDevelop, kfunk, mwolff

Subscribers: mwolff, kfunk, kdevelop-devel

Differential Revision: https://phabricator.kde.org/D4435

Instead, it should be:

Feature: Add file template for QObject with pimpl

The current QObject template has all private members exposed
in the class, and there are reasons to use a pimpl variant
instead.
As the current file template logic does not allow to control
the number of files generated (like only generating the private
header if the user toggles a config option), the QObject pimpl
template is a complete separate template.

Both QObject templates share template snippets where useful.

Differential Revision: https://phabricator.kde.org/D4435
  • no "Summary:" title
  • no "Reviewers", "Subscribers": These two hold no meaning whatsoever in a git context and are only relevant for phabricator
  • no "Reviewed By": This could be in there, but personally I also see no value in this as this information is stored on phabricator in the differential URL

Another reason for having a template:

presetting the list of reviewers when sending a new patch via arc diff. I.e. all patches to repository A should get reviewers #foo set, stuff like that.

@milianw Please keep in mind Phabricator is an enterprise tool, and the majority of installs (99%) are businesses who rely on the accountability we've built into the software.

@chad sorry, but I don't follow, what has accountability to do with this?

I was commenting on your suggestion to remove unwanted items from commit messages. It's a compliance feature for many businesses who need complete audit trails.

Note that I'm not asking for this to be done for everyone, by default. Rather, I want this to be an opt-in feature. I imagine this is also what the people who voted for the question on StackOverflow have in mind.

Now, looking at this: https://secure.phabricator.com/w/guides/arcanist_workflows/#facebook-workflow <-- It seems there is already support for custom templates, i.e.:

echo '{"edit":"create"}' | arc call-conduit differential.getcommitmessage
{"error":null,"errorMessage":null,"response":"<<Replace this line with your revision title>\n\nSummary: \n\nTest Plan: \n\nReviewers: \n\nSubscribers: "}

So can this already be configured on the server side somehow?