[PATCH] Add commit message guidelines to developer policy

Renato Golin renato.golin at linaro.org
Mon Mar 16 13:36:41 PDT 2015


================
Comment at: docs/DeveloperPolicy.rst:320-322
@@ +319,5 @@
+* `Attribution of Changes`_ should be in a separate line, after the end of
+  the body, as simple as "Patch by John Doe.". This is how we officially
+  handle attribution, and there are automated processes that rely on this
+  format.
+
----------------
rsmith wrote:
> rengolin wrote:
> > rsmith wrote:
> > > rengolin wrote:
> > > > rsmith wrote:
> > > > > If we want to officially support such processes, we should be very explicit as to exactly what format they're looking for. Presumably, it's a case-insensitive match for the string "patch by "?
> > > > I'm not being specific about the process of identifying it for a very special reason: we don't need to.
> > > > 
> > > > Regular expressions can be quite powerful in detecting patters, and any current script can already deal with all the variations we already have, so there's no reason to be explicit on the format. However, names can be very hard to parse, especially international names or abbreviations, and trying to get a consensus on that would be impossible.
> > > > 
> > > > So we rely on common sense. The most used "automated" process right now is to "grep for someone's name on the git log". So what "patch by" string looks like is irrelevant, including capitalization, exclamation marks, or others. 
> > > Yours is not the only use case. "grep for someone's name on the git log" doesn't let us find all patches authored by anyone who is not the committer, which is sometimes a useful thing to do. (It's also liable to fail if there are variations in how the name is spelled.)
> > > 
> > > If we want to allow automated processes to identify information in commit messages, we should precisely specify how such information should be formatted / detected. What's the point in having a policy on this if we don't actually make it precise enough to be useful?
> > This is not a policy, it's a guideline, and the consensus was to make it vague on purpose. If people are really going to nit pick on every single detail, than maybe we should just scrap the whole thing and go back to what it was before, ie, no guideline.
> > 
> > We wrote this to help people write better commit messages, not to force them through a git pre-commit hook or anything of the sort.
> > 
> > Maybe I should just remove the note about automated process...
> The attribution part of this used to be policy rather than guidelines, and should remain that way. Perhaps it shouldn't have been moved to this list of guidelines?
Sorry Richard, you're a bit late on the discussion. The "policy" was very vague and didn't reflect reality. You can see how I deleted the "policy" from below in this patch, because that was the consensus we reached.

The reason, as you can read back on this review or on the mailing list, was that there were already many variations of the attribution pattern and favouring one over others didn't make sense. Furthermore, the most used "automation" was to grep for a user's name, which makes any discussion on how we write "patch by" irrelevant.

The final consensus was that writing "patch by foo" would give people the meaning (since this is a guideline), but the first line means that you *should* attribute to a third party if you didn't write the patch. The existence is needed, the format is loose.

http://reviews.llvm.org/D8197

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list