[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Clarify Crediting

From: <kfogel_at_collab.net>
Date: 2005-08-04 17:56:49 CEST

Joshua Varner <jlvarner@gmail.com> writes:
> 1.) For non-committers should e-mail's always be in <>'s, even if there is no
> associated name?

Yes, please.

> 2.) Can parenthetical asides follow after a name, within the same heading?
> Like this:
>
> Suggestion By: Fred <fred@fred.com>
> (Reported the problem.)
> Joe <joe@joe.com>
> (Created the reproduction recipe.)

No, let's please limit it to one parenthetical aside per field, coming
immediately after the entire (possibly multi-line) field. I've
changed the example to show this.

(I actually have some parsing code almost done for this format, and
will post it as soon as I can.)

> [[[
>
> * www/hacking.html
> (crediting): Clarify when you can use a username or name/e-mail.
> Additional minor wording/grammar tweaks.
>
> ]]]

Great! I've committed your patch, with some tweaks, see below.

> Index: hacking.html
> ===================================================================
> --- hacking.html (revision 15458)
> +++ hacking.html (working copy)
> @@ -1430,14 +1430,15 @@
> <h2>Crediting</h2>

Please generate the patch from the top of your trunk tree, so that it
applies to "www/hacking.html". This allows the applier to always do
the same thing to get the right result.

> <p>It is very important to record code contributions in a consistent
> -and parseable way. This allows us to write scripts which help us
> -figure out who has been actively contributing and how, so we can spot
> -potential new committers quickly. The Subversion project uses
> -human-readable but machine-parseable fields in log messages for this,
> -as described below.</p>
> +and parseable way. This allows us to write scripts to figure out who
> +has been actively contributing&nbsp;&mdash;&nbsp;and what they have
> +contributed&nbsp;&mdash;&nbsp;so we can spot potential new committers
> +quickly. The Subversion project uses human-readable but
> +machine-parseable fields in log messages to accomplish this.</p>
>
> <p>When committing a patch written by someone else, use
> -"Patch&nbsp;by:&nbsp;" at the beginning of a line:</p>
> +"Patch&nbsp;by:&nbsp;" at the beginning of a line to indicate the
> +author</p>
>
> <pre>
> Fix issue #1729: Don't crash because of a missing file.
> @@ -1448,24 +1449,29 @@
> (frobnicate_file): Check that file exists before frobnicating.
> </pre>
>
> -<p>If multiple people wrote the patch, name them all, one person per
> -line, making sure to start each continuation line with whitespace. If
> -you (the committer) were one of the people, list yourself as "me".
> -Thus:</p>
> +<p>If multiple individuals wrote the patch, list them each on a
> +separate line&nbsp;&mdash;&nbsp;making sure to start each continuation
> +line with whitespace. Non-committers should be listed by name, if
> +known, and e-mail. Committers may be listed similarly, or by their
> +canonical usernames from COMMITTERS (the leftmost
> +column). Additionally, "me" is an acceptable shorthand for the person
> +actually committing the change.</p>

Totally minor nit: two spaces after a sentence-ending period. The
rest of the entry uses that style, and it makes sentence-motion
commands in editors possible. Not a big deal, though, and certainly
no patch should ever be rejected over it.

> <pre>
> Fix issue #1729: Don't crash because of a missing file.
>
> Patch by: J. Random &lt;jrandom@example.com&gt;
> Enrico Caruso &lt;codingtenor@codingtenor.com&gt;
> + jcommitter
> me
>
> * subversion/libsvn_ra_ansible/get_editor.c
> (frobnicate_file): Check that file exists before frobnicating.
> </pre>
>
> -<p>If someone pointed out a problem or suggested the fix, but didn't
> -actually write the patch, use "Suggested&nbsp;by:&nbsp;":</p>
> +<p>If someone pointed out a problem or suggested the fix, but did not
> +actually write the patch, indicate the contribution with
> +"Suggested&nbsp;by:&nbsp;":</p>
>
> <pre>
> Fix issue #1729: Don't crash because of a missing file.
> @@ -1476,7 +1482,7 @@
> (frobnicate_file): Check that file exists before frobnicating.
> </pre>
>
> -<p>If someone helped review the change, use "Review&nbsp;by:&nbsp;"</p>
> +<p>If someone reviewed the change, use "Review&nbsp;by:&nbsp;"</p>
>
> <pre>
> Fix issue #1729: Don't crash because of a missing file.
> @@ -1488,12 +1494,9 @@
>
> </pre>
>
> -<p>(As with "Patch&nbsp;by:&nbsp;", you can name multiple people in
> -"Review&nbsp;by:&nbsp;" or "Suggested&nbsp;by:&nbsp;" via
> -whitespace-prefixed continuation lines.)</p>
> +<p> All three fields may have multiple lines, and log messages may
> +contain any combination of the fields.
>
> -<p>Multiple fields in the same log message are fine, for example:</p>
> -

Good, but you lost the closing "</p>" tag here. I put it back.

> <pre>
> Fix issue #1729: Don't crash because of a missing file.
>
> @@ -1502,13 +1505,14 @@
> me
> Suggested by: J. Random &lt;jrandom@example.com&gt;
> Review by: Eagle Eyes &lt;eeyes@example.com&gt;
> + jcommitter
>
> * subversion/libsvn_ra_ansible/get_editor.c
> (frobnicate_file): Check that file exists before frobnicating.
> </pre>
>
> -<p>To give further details about a contribution, use a parenthetical
> -aside immediately after that field, for example:</p>
> +<p>Further details about a contribution may be listed in a
> +parenthetical aside immediately after that field.</p>
>
> <pre>
> Fix issue #1729: Don't crash because of a missing file.
> @@ -1521,14 +1525,7 @@
> </pre>
>
> <p>It is understood that a parenthetical aside immediately following a
> -field applies to that field, and that "me" refers to person who
> -committed this revision. You don't have to write "me", you can use
> -your name instead, if you're not tired of typing it. Also, although
> -the examples above use full name and email address, you can use a
> -committer's username to refer to that committer from any field. For
> -example, "Philip Martin &lt;philip@codematters.co.uk&gt;" and "philip"
> -would be equivalent. See the leftmost column of the COMMITTERS file
> -for canonical usernames.</p>
> +field applies to that field.
>
> <p>Currently, these three fields</p>

Same about the "</p>" tag.

> @@ -1538,17 +1535,16 @@
> Review by:
> </pre>
>
> -<p>are the only officially-supported crediting fields (where
> +<p>are the only officially supported crediting fields (where

In "A Dictionary of Modern English Usage", Henry Fowler says in his
extensive section on the hyphen:

   "The chaos prevailing among writers or printers or both regarding
    the use of hyphens is discreditable to English education. Since
    it sufficiently proves by its existence that neither the
    importance of proper hyphening nor the way to set about it is
    commonly known, this article may well begin with a dozen examples,
    all taken faithfully from newspapers, in which the wrong use or
    wrong non-use of hyphens makes the words, if strictly interpreted,
    mean something different from what the writers intended. It is no
    adequate answer to such criticism to say that actual
    misunderstanding is unlikely; to have to depend on one's
    employer's readiness to take the will for the deed is surely a
    humliation that no decent craftsman should be willing to put up
    with..."

The subsection that covers this particular use is (E), in which he
recommends a subtle but sound formula for this case. He says the
hyphen may be used

   ...To attach closely to an active or passive participle an adverb
   or preposition preceding or following it that would not require
   hyphening to the parent verb (you "put up", not "put-up", a job,
   but the result is a "put-up" job). The question whether this
   hyphening is to be done or not is answered, as in B, by the
   accentuation; the hyphen is wrong unless the compound will have
   only one accent, & that on the first element; thus "oft-repeated"
   will usually be hypened, & "ill served" usually not.

   ...The guiding principles will be: No hyphening of words that will
   do as well separate; no hyphening of words in the B or E class if
   they retain the normal accentuation; ...

Although I do not normally think authority has much to offer in
discussions of language usage, I'd follow Fowler off a cliff, and
therefore agree with your removal of the hyphen from "officially
supported".

Thank you. This mail has been a great pleasure to write :-).

> "supported" means scripts know to look for them), and they are widely
> -used in Subversion log messages. Future fields would probably also be
> -of the form "VERB&nbsp;by:&nbsp;", and from time to time someone may
> -use a field that sounds official but really
> -isn't&nbsp;&mdash;&nbsp;there are a few instances of
> -"Reported&nbsp;by:&nbsp;", for example. These are okay, but try to
> -use an official field, or a parenthetical aside, in preference to
> -making up your own field. Also, don't use "Reported&nbsp;by:&nbsp;"
> -when the reporter is already recorded in an issue; instead, just refer
> -to the issue.</p>
> +used in Subversion log messages. Future fields will probably be of
> +the form "VERB&nbsp;by:&nbsp;", and from time to time someone may use
> +a field that sounds official but really is not&nbsp;&mdash;&nbsp;for
> +example, there are a few instances of "Reported&nbsp;by:&nbsp;".
> +These are okay, but try to use an official field, or a parenthetical
> +aside, in preference to creating your own. Also, don't use
> +"Reported&nbsp;by:&nbsp;" when the reporter is already recorded in an
> +issue; instead, simply refer to the issue.</p>
>
> <p>Look over Subversion's existing log messages to see how to use
> these fields in practice. This command from the top of your trunk
> @@ -1558,12 +1554,12 @@
> svn log | contrib/client-side/search-svnlog.pl "(Patch|Review|Suggested) by: "
> </pre>
>
> -<p>Note that the "Approved&nbsp;by:&nbsp;" field seen in some commits
> -is totally unrelated to these crediting fields (and is rarely parsed
> -by scripts). It is simply the standard syntax for indicating either
> -who approved a partial committer's commit outside their usual area, or
> -(in the case of merges to release branches) who voted for the change
> -to be merged.</p>
> +<p> <b>Note:</b> The "Approved&nbsp;by:&nbsp;" field seen in some
> +commit messages is totally unrelated to these crediting fields (and is
> +rarely parsed by scripts). It is simply the standard syntax for
> +indicating either who approved a partial committer's commit outside
> +their usual area, or (in the case of merges to release branches) who
> +voted for the change to be merged.</p>
>
> </div>

The rest looks good. Committed with a few tweaks in r15587.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 4 18:51:30 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.