[PATCH 0/3] Update SubmittingPatches

[PATCH 0/3] Update SubmittingPatches

[Junio C Hamano]

The main thing this series wants to do is the second one, but I
wanted to reduce the clutter in this document while at it.

Junio C Hamano (3):
  SubmittingPatches: who am I and who cares?
  SubmittingPatches: mention subsystems with dedicated repositories
  SubmittingPatches: remove overlong checklist

 Documentation/SubmittingPatches | 160 ++++++++++++++++++++--------------------
 1 file changed, 79 insertions(+), 81 deletions(-)

-- 
1.8.1.209.gc32ab23

[PATCH 1/3] SubmittingPatches: who am I and who cares?

[Junio C Hamano]

The introductory text in the "long version" talks about the origin
of this document with "I started ...", but it is unclear who that I
is, and more importantly, it is not interesting how it was started.

Just state the purpose of the document to help readers decide if it
is releavant to them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/SubmittingPatches | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c34c9d1..b9fd182 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -60,14 +60,8 @@ Checklist (and a short version for the impatient):
 
 Long version:
 
-I started reading over the SubmittingPatches document for Linux
-kernel, primarily because I wanted to have a document similar to
-it for the core GIT to make sure people understand what they are
-doing when they write "Signed-off-by" line.
-
-But the patch submission requirements are a lot more relaxed
-here on the technical/contents front, because the core GIT is
-thousand times smaller ;-).  So here is only the relevant bits.
+Here are some guidelines for people who want to contribute their code
+to this software.
 
 (0) Decide what to base your work on.
 
-- 
1.8.1.209.gc32ab23

[PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories

[Junio C Hamano]

These were only mentioned in periodical "A note from the maintainer"
posting and not in the documentation suite.  SubmittingPatches has a
section to help contributors decide on what commit to base their
changes, which is the most suitable place for this information.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/SubmittingPatches | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index b9fd182..e810263 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -88,6 +88,10 @@ change is relevant to.
    wait until some of the dependent topics graduate to 'master', and
    rebase your work.
 
+ - Some parts of the system have dedicated maintainers with their own
+   repositories (see the section "Subsystems" below).  Changes to
+   these parts should be based on their trees.
+
 To find the tip of a topic branch, run "git log --first-parent
 master..pu" and look for the merge commit. The second parent of this
 commit is the tip of the topic branch.
@@ -279,6 +283,26 @@ You can also create your own tag or use one that's in common usage
 such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
 
 ------------------------------------------------
+Subsystems with dedicated maintainers
+
+Some parts of the system have dedicated maintainers with their own
+repositories.
+
+ - git-gui/ comes from git-gui project, maintained by Pat Thoyts:
+
+        git://repo.or.cz/git-gui.git
+
+ - gitk-git/ comes from Paul Mackerras's gitk project:
+
+        git://ozlabs.org/~paulus/gitk
+
+ - po/ comes from the localization coordinator, Jiang Xin:
+
+	https://github.com/git-l10n/git-po/
+
+Patches to these parts should be based on their trees.
+
+------------------------------------------------
 An ideal patch flow
 
 Here is an ideal patch flow for this project the current maintainer
-- 
1.8.1.209.gc32ab23

[PATCH 3/3] SubmittingPatches: remove overlong checklist

[Junio C Hamano]

The section is no longer a concise checklist.  It also talks about
things that are not covered in the "Long version" text, which means
people need to read both, covering more or less the same thing in
different phrasing.

Fold the details into the main text and remove the section.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/SubmittingPatches | 126 +++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 73 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index e810263..a7daad3 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -1,65 +1,3 @@
-Checklist (and a short version for the impatient):
-
-	Commits:
-
-	- make commits of logical units
-	- check for unnecessary whitespace with "git diff --check"
-	  before committing
-	- do not check in commented out code or unneeded files
-	- the first line of the commit message should be a short
-	  description (50 characters is the soft limit, see DISCUSSION
-	  in git-commit(1)), and should skip the full stop
-	- it is also conventional in most cases to prefix the
-	  first line with "area: " where the area is a filename
-	  or identifier for the general area of the code being
-	  modified, e.g.
-	  . archive: ustar header checksum is computed unsigned
-	  . git-cherry-pick.txt: clarify the use of revision range notation
-	  (if in doubt which identifier to use, run "git log --no-merges"
-	  on the files you are modifying to see the current conventions)
-	- the body should provide a meaningful commit message, which:
-	  . explains the problem the change tries to solve, iow, what
-	    is wrong with the current code without the change.
-	  . justifies the way the change solves the problem, iow, why
-	    the result with the change is better.
-	  . alternate solutions considered but discarded, if any.
-	- describe changes in imperative mood, e.g. "make xyzzy do frotz"
-	  instead of "[This patch] makes xyzzy do frotz" or "[I] changed
-	  xyzzy to do frotz", as if you are giving orders to the codebase
-	  to change its behaviour.
-	- try to make sure your explanation can be understood without
-	  external resources. Instead of giving a URL to a mailing list
-	  archive, summarize the relevant points of the discussion.
-	- add a "Signed-off-by: Your Name <you@example.com>" line to the
-	  commit message (or just use the option "-s" when committing)
-	  to confirm that you agree to the Developer's Certificate of Origin
-	- make sure that you have tests for the bug you are fixing
-	- make sure that the test suite passes after your commit
-
-	Patch:
-
-	- use "git format-patch -M" to create the patch
-	- do not PGP sign your patch
-	- do not attach your patch, but read in the mail
-	  body, unless you cannot teach your mailer to
-	  leave the formatting of the patch alone.
-	- be careful doing cut & paste into your mailer, not to
-	  corrupt whitespaces.
-	- provide additional information (which is unsuitable for
-	  the commit message) between the "---" and the diffstat
-	- if you change, add, or remove a command line option or
-	  make some other user interface change, the associated
-	  documentation should be updated as well.
-	- if your name is not writable in ASCII, make sure that
-	  you send off a message in the correct encoding.
-	- send the patch to the list (git@vger.kernel.org) and the
-	  maintainer (gitster@pobox.com) if (and only if) the patch
-	  is ready for inclusion. If you use git-send-email(1),
-	  please test it first by sending email to yourself.
-	- see below for instructions specific to your mailer
-
-Long version:
-
 Here are some guidelines for people who want to contribute their code
 to this software.
 
@@ -119,13 +57,52 @@ change, the approach taken by the change, and if relevant how this
 differs substantially from the prior version, are all good things
 to have.
 
+Make sure that you have tests for the bug you are fixing.
+
+When adding a new feature, make sure that you have new tests to show
+the feature triggers the new behaviour when it should, and to show the
+feature does not trigger when it shouldn't.  Also make sure that the
+test suite passes after your commit.  Do not forget to update the
+documentation to describe the updated behaviour.
+
 Oh, another thing.  I am picky about whitespaces.  Make sure your
 changes do not trigger errors with the sample pre-commit hook shipped
 in templates/hooks--pre-commit.  To help ensure this does not happen,
 run git diff --check on your changes before you commit.
 
+(2) Describe your changes well.
+
+The first line of the commit message should be a short description (50
+characters is the soft limit, see DISCUSSION in git-commit(1)), and
+should skip the full stop.  It is also conventional in most cases to
+prefix the first line with "area: " where the area is a filename or
+identifier for the general area of the code being modified, e.g.
+
+  . archive: ustar header checksum is computed unsigned
+  . git-cherry-pick.txt: clarify the use of revision range notation
+
+If in doubt which identifier to use, run "git log --no-merges" on the
+files you are modifying to see the current conventions.
+
+The body should provide a meaningful commit message, which:
 
-(2) Generate your patch using git tools out of your commits.
+  . explains the problem the change tries to solve, iow, what is wrong
+    with the current code without the change.
+
+  . justifies the way the change solves the problem, iow, why the
+    result with the change is better.
+
+  . alternate solutions considered but discarded, if any.
+
+Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
+instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
+to do frotz", as if you are giving orders to the codebase to change
+its behaviour.  Try to make sure your explanation can be understood
+without external resources. Instead of giving a URL to a mailing list
+archive, summarize the relevant points of the discussion.
+
+
+(3) Generate your patch using git tools out of your commits.
 
 git based diff tools generate unidiff which is the preferred format.
 
@@ -133,22 +110,27 @@ You do not have to be afraid to use -M option to "git diff" or
 "git format-patch", if your patch involves file renames.  The
 receiving end can handle them just fine.
 
-Please make sure your patch does not include any extra files
-which do not belong in a patch submission.  Make sure to review
+Please make sure your patch does not add commented out debugging code,
+or include any extra files which do not relate to what your patch
+is trying to achieve. Make sure to review
 your patch after generating it, to ensure accuracy.  Before
 sending out, please make sure it cleanly applies to the "master"
 branch head.  If you are preparing a work based on "next" branch,
 that is fine, but please mark it as such.
 
 
-(3) Sending your patches.
+(4) Sending your patches.
 
 People on the git mailing list need to be able to read and
 comment on the changes you are submitting.  It is important for
 a developer to be able to "quote" your changes, using standard
 e-mail tools, so that they may comment on specific portions of
 your code.  For this reason, all patches should be submitted
-"inline".  WARNING: Be wary of your MUAs word-wrap
+"inline".  If your log message (including your name on the
+Signed-off-by line) is not writable in ASCII, make sure that
+you send off a message in the correct encoding.
+
+WARNING: Be wary of your MUAs word-wrap
 corrupting your patch.  Do not cut-n-paste your patch; you can
 lose tabs that way if you are not careful.
 
@@ -200,16 +182,14 @@ patch, format it as "multipart/signed", not a text/plain message
 that starts with '-----BEGIN PGP SIGNED MESSAGE-----'.  That is
 not a text/plain, it's something else.
 
-Unless your patch is a very trivial and an obviously correct one,
-first send it with "To:" set to the mailing list, with "cc:" listing
+Send your patch with "To:" set to the mailing list, with "cc:" listing
 people who are involved in the area you are touching (the output from
 "git blame $path" and "git shortlog --no-merges $path" would help to
 identify them), to solicit comments and reviews.  After the list
 reached a consensus that it is a good idea to apply the patch, re-send
-it with "To:" set to the maintainer and optionally "cc:" the list for
-inclusion.  Do not forget to add trailers such as "Acked-by:",
-"Reviewed-by:" and "Tested-by:" after your "Signed-off-by:" line as
-necessary.
+it with "To:" set to the maintainer and "cc:" the list for inclusion.
+Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
+"Tested-by:" after your "Signed-off-by:" line as necessary.
 
 
 (4) Sign your work
-- 
1.8.1.209.gc32ab23

Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories

[Jason Holden]

On Tue, Jan 01, 2013 at 03:24:54PM -0800, Junio C Hamano wrote:
>  
>  ------------------------------------------------
> +Subsystems with dedicated maintainers
> +
> +Some parts of the system have dedicated maintainers with their own
> +repositories.
> +
> + - git-gui/ comes from git-gui project, maintained by Pat Thoyts:
> +
> +        git://repo.or.cz/git-gui.git
> +
> + - gitk-git/ comes from Paul Mackerras's gitk project:
> +
> +        git://ozlabs.org/~paulus/gitk
> +
> + - po/ comes from the localization coordinator, Jiang Xin:
> +
> +	https://github.com/git-l10n/git-po/
> +
> +Patches to these parts should be based on their trees.
> +
> +------------------------------------------------
>  An ideal patch flow
>  

Any reason to leave out the maintainers email addresses? If we add that, all
the content of the MAINTAINERS file we had been discussing would then be in
this file.

My only other suggestion for this series might be to augment the file with 
a patch submittal example(s).  I found the best example to be in 
git-send-email's man page, but maybe enumerate the example out for the 
most common workflows.  Maybe something like:

-----------------------------------------------------------
Example of sending patches for a new feature:

Create the patches:
 $ git format-patch --cover-letter -M origin/master -o outgoing/
 $ edit outgoing/0000-*

To send your completed patches for initial consideration:
 $ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com

To send your reviewed patches for inclusion:
 $ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org

-Jason

Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories

[Junio C Hamano]

Jason Holden <jason.k.holden.swdev@gmail.com> writes:

> Any reason to leave out the maintainers email addresses?

Nothing particular, other than that I did not find anywhere in the
file that does not break the flow.

> My only other suggestion for this series might be to augment the file with 
> a patch submittal example(s).  I found the best example to be in 
> git-send-email's man page,...

I'd feel better to avoid copying and pasting.  If send-email has
good examples, shouldn't it be sufficient to refer to them?

> -----------------------------------------------------------
> Example of sending patches for a new feature:
>
> Create the patches:
>  $ git format-patch --cover-letter -M origin/master -o outgoing/
>  $ edit outgoing/0000-*
>
> To send your completed patches for initial consideration:
>  $ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com

This is ambiguous; it makes it look as if you are CC'ing the
maintainer, but for the initial round, it is likely that you are
sending it to me only because I have been involved in the area the
patch touches.

> To send your reviewed patches for inclusion:
>  $ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org

This is fine, but we would probably want to see it CC'ed to people
who reviewed the initial submission, too.

Re: [PATCH 0/3] Update SubmittingPatches

[Jeff King]

On Tue, Jan 01, 2013 at 03:24:52PM -0800, Junio C Hamano wrote:

> The main thing this series wants to do is the second one, but I
> wanted to reduce the clutter in this document while at it.
> 
> Junio C Hamano (3):
>   SubmittingPatches: who am I and who cares?
>   SubmittingPatches: mention subsystems with dedicated repositories
>   SubmittingPatches: remove overlong checklist

FWIW, all look reasonable to me. I did not pay close attention to the
earlier discussion on MAINTAINERS, but my first thought on reading it
was "why isn't this stuff just in the SubmittingPatches doc? That's
where I would look for it".

I guess some people might balk at the third one, as the checklist is
supposed to help with a quick and easy start. But I agree with your
commit log message that it isn't short or quick anymore. Of course it
has been a long time since I have needed to read that document. Maybe
people who have used it more recently would have more relevant thoughts.

-Peff

Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jason Holden <jason.k.holden.swdev@gmail.com> writes:
>
>> Any reason to leave out the maintainers email addresses?
>
> Nothing particular, other than that I did not find anywhere in the
> file that does not break the flow.

I've prepared this on top of the previous three.  The second hunk
that updates "(4) Sending your patches." is the logical place to
have this information.

The other hunks are correcting minor mistakes that are not related.
I think I'll squash them to the patch 3/3.


 Documentation/SubmittingPatches | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index a7daad3..9e113d0 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -31,8 +31,9 @@ change is relevant to.
    these parts should be based on their trees.
 
 To find the tip of a topic branch, run "git log --first-parent
-master..pu" and look for the merge commit. The second parent of this
-commit is the tip of the topic branch.
+master..pu" and look for merge commits. The second parent of these
+commits are the tips of topic branches.
+
 
 (1) Make separate commits for logically separate changes.
 
@@ -70,6 +71,7 @@ changes do not trigger errors with the sample pre-commit hook shipped
 in templates/hooks--pre-commit.  To help ensure this does not happen,
 run git diff --check on your changes before you commit.
 
+
 (2) Describe your changes well.
 
 The first line of the commit message should be a short description (50
@@ -185,14 +187,20 @@ not a text/plain, it's something else.
 Send your patch with "To:" set to the mailing list, with "cc:" listing
 people who are involved in the area you are touching (the output from
 "git blame $path" and "git shortlog --no-merges $path" would help to
-identify them), to solicit comments and reviews.  After the list
-reached a consensus that it is a good idea to apply the patch, re-send
-it with "To:" set to the maintainer and "cc:" the list for inclusion.
-Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
-"Tested-by:" after your "Signed-off-by:" line as necessary.
+identify them), to solicit comments and reviews.
+
+After the list reached a consensus that it is a good idea to apply the
+patch, re-send it with "To:" set to the maintainer [*1*] and "cc:" the
+list [*2*] for inclusion.  Do not forget to add trailers such as
+"Acked-by:", "Reviewed-by:" and "Tested-by:" after your
+"Signed-off-by:" line as necessary.
+
+    [Addresses]
+     *1* The current maintainer: gitster@pobox.com
+     *2* The mailing list: git@vger.kernel.org
 
 
-(4) Sign your work
+(5) Sign your work
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches
@@ -304,7 +312,8 @@ suggests to the contributors:
      even get them in a "on top of your change" patch form.
 
  (3) Polish, refine, and re-send to the list and the people who
-     spend their time to improve your patch.  Go back to step (2).
+     spent their time to help improving your patch.  Go back to
+     step (2) until step (4) happens.
 
  (4) The list forms consensus that the last round of your patch is
      good.  Send it to the list and cc the maintainer.

[PATCH] SubmittingPatches: Document how to request a patch review tag

[Jason Holden]

Document the preferred way a developer should request to have their
Acked-by/Tested-by/Reviewed-by tag to a patch series under discussion

Signed-off-by: Jason Holden <jason.k.holden.swdev@gmail.com>
---
Junio,
  I was ready to add my Reviewed-by to this patch series, but I wasn't sure if
I should email just you the patch author (to cut down on overall list traffic)
or both you and the list.  If all reviewed-by/acked-by/tested-by traffic 
should go via the email list I think this patch would be helpful, as I 
wasn't quite sure how wide of a distribution list to use for my 
"Reviewed-by" email.

A very similiar question was asked previously in:
http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570

This will apply on top of your last tweak to SubmittingPatches

Please add my reviewed-by to the rest of the patches in this series.

-Jason

 Documentation/SubmittingPatches | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f6276ff..80001c9 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -268,6 +268,11 @@ If you like, you can put extra tags at the end:
 4. "Tested-by:" is used to indicate that the person applied the patch
    and found it to have the desired effect.
 
+If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
+to a patch series under discussion (after having reviewed it or tested it
+of course!), reply to the author of the patch series, cc'ing the git mailing
+list.
+
 You can also create your own tag or use one that's in common usage
 such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
 
-- 
1.8.1.rc3.28.g0ab5d1f

Re: [PATCH] SubmittingPatches: Document how to request a patch review tag

[Junio C Hamano]

Jason Holden <jason.k.holden.swdev@gmail.com> writes:

> A very similiar question was asked previously in:
> http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570

"Reviewed-by" is for those who are familiar with the part of the
system being touched to say "I reviewed this patch, it looks good",
and Michael indeed was involved in recent updates to the refs.c
infrastructure, so as he said in his message "it looks like I should",
it was the right thing to do.

I do not think Michael was asking if that was the standard _thing_
to do; I think the question was if there was a standard _way_
(perhaps a tool) to send such a "Reviewed-by:" line.

> This will apply on top of your last tweak to SubmittingPatches
>
> Please add my reviewed-by to the rest of the patches in this series.

I do not think you "own" anyting in SubmittingPatches document,
though; at least not yet.

Re: [PATCH] SubmittingPatches: Document how to request a patch review tag

[Michael Haggerty]

On 01/04/2013 09:58 PM, Jason Holden wrote:
> Document the preferred way a developer should request to have their
> Acked-by/Tested-by/Reviewed-by tag to a patch series under discussion
> 
> Signed-off-by: Jason Holden <jason.k.holden.swdev@gmail.com>
> ---
> Junio,
>   I was ready to add my Reviewed-by to this patch series, but I wasn't sure if
> I should email just you the patch author (to cut down on overall list traffic)
> or both you and the list.  If all reviewed-by/acked-by/tested-by traffic 
> should go via the email list I think this patch would be helpful, as I 
> wasn't quite sure how wide of a distribution list to use for my 
> "Reviewed-by" email.
> 
> A very similiar question was asked previously in:
> http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570

On 01/04/2013 10:47 PM, Junio C Hamano wrote:
> "Reviewed-by" is for those who are familiar with the part of the
> system being touched to say "I reviewed this patch, it looks good",
> and Michael indeed was involved in recent updates to the refs.c
> infrastructure, so as he said in his message "it looks like I should",
> it was the right thing to do.
>
> I do not think Michael was asking if that was the standard _thing_
> to do; I think the question was if there was a standard _way_
> (perhaps a tool) to send such a "Reviewed-by:" line.

Junio is correct; I was just asking whether there was a particular email
convention for adding a "Reviewed-by:" line.  It would be nice for this
to be mentioned in the documentation.

>  Documentation/SubmittingPatches | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index f6276ff..80001c9 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -268,6 +268,11 @@ If you like, you can put extra tags at the end:
>  4. "Tested-by:" is used to indicate that the person applied the patch
>     and found it to have the desired effect.
>  
> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
> +to a patch series under discussion (after having reviewed it or tested it
> +of course!), reply to the author of the patch series, cc'ing the git mailing
> +list.
> +
>  You can also create your own tag or use one that's in common usage
>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".

I don't think this is quite correct.  Such emails should be
"reply-to-all" people who have participated in the thread, which might
include more than just the patch author and the git mailing list.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH] SubmittingPatches: Document how to request a patch review tag

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 01/04/2013 10:47 PM, Junio C Hamano wrote:
>> "Reviewed-by" is for those who are familiar with the part of the
>> system being touched to say "I reviewed this patch, it looks good",
>> and Michael indeed was involved in recent updates to the refs.c
>> infrastructure, so as he said in his message "it looks like I should",
>> it was the right thing to do.
>>
>> I do not think Michael was asking if that was the standard _thing_
>> to do; I think the question was if there was a standard _way_
>> (perhaps a tool) to send such a "Reviewed-by:" line.
>
> Junio is correct; I was just asking whether there was a particular email
> convention for adding a "Reviewed-by:" line.  It would be nice for this
> to be mentioned in the documentation.

Yeah, I wasn't exactly correct in that I was talking more about
Acked-by than Reviewed-by, but they are morally very similar and the
same argument applies to both.

In the particular case of your "refs.c" review, because you are not
just familiar with the code, but essentially are the current owner
of the code, Acked-by might have been even more appropriate than
Reviewed-by, by the way.

>> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
>> +to a patch series under discussion (after having reviewed it or tested it
>> +of course!), reply to the author of the patch series, cc'ing the git mailing
>> +list.
>> +
>>  You can also create your own tag or use one that's in common usage
>>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
>
> I don't think this is quite correct.  Such emails should be
> "reply-to-all" people who have participated in the thread, which might
> include more than just the patch author and the git mailing list.

That would be more helpful.  In practice, I can pick these up either
way, but Cc'ing everybody would be better as a common courtesy.

When the author resubmits an already reviewed patch with these Acks
and Reviews for final application, these reviewers should be Cc'ed
so that they can say "Huh?  that is not the exact patch I reviewed.
What is going on?".

Thanks for a review.