{"thread":{"id":"32506","subject":"[PATCH 0/3] Update SubmittingPatches","startedAt":"2013-01-01T23:24:52Z","lastAt":"2013-01-06T09:01:42Z","messageCount":12,"participants":["Junio C Hamano","Jason Holden","Jeff King","Michael Haggerty"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"205811","messageId":"1357082695-29713-1-git-send-email-gitster@pobox.com","threadId":"32506","inReplyTo":null,"subject":"[PATCH 0/3] Update SubmittingPatches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T23:24:52Z","receivedAt":"2013-01-01T23:24:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The main thing this series wants to do is the second one, but I\nwanted to reduce the clutter in this document while at it.\n\nJunio C Hamano (3):\n SubmittingPatches: who am I and who cares?\n SubmittingPatches: mention subsystems with dedicated repositories\n SubmittingPatches: remove overlong checklist\n\n Documentation/SubmittingPatches | 160 ++++++++++++++++++++--------------------\n 1 file changed, 79 insertions(+), 81 deletions(-)\n\n-- \n1.8.1.209.gc32ab23\n"},{"id":"205812","messageId":"1357082695-29713-2-git-send-email-gitster@pobox.com","threadId":"32506","inReplyTo":"1357082695-29713-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] SubmittingPatches: who am I and who cares?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T23:24:53Z","receivedAt":"2013-01-01T23:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The introductory text in the \"long version\" talks about the origin\nof this document with \"I started ...\", but it is unclear who that I\nis, and more importantly, it is not interesting how it was started.\n\nJust state the purpose of the document to help readers decide if it\nis releavant to them.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/SubmittingPatches | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex c34c9d1..b9fd182 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -60,14 +60,8 @@ Checklist (and a short version for the impatient):\n \n Long version:\n \n-I started reading over the SubmittingPatches document for Linux\n-kernel, primarily because I wanted to have a document similar to\n-it for the core GIT to make sure people understand what they are\n-doing when they write \"Signed-off-by\" line.\n-\n-But the patch submission requirements are a lot more relaxed\n-here on the technical/contents front, because the core GIT is\n-thousand times smaller ;-). So here is only the relevant bits.\n+Here are some guidelines for people who want to contribute their code\n+to this software.\n \n (0) Decide what to base your work on.\n \n-- \n1.8.1.209.gc32ab23\n"},{"id":"205813","messageId":"1357082695-29713-3-git-send-email-gitster@pobox.com","threadId":"32506","inReplyTo":"1357082695-29713-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T23:24:54Z","receivedAt":"2013-01-01T23:24:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These were only mentioned in periodical \"A note from the maintainer\"\nposting and not in the documentation suite. SubmittingPatches has a\nsection to help contributors decide on what commit to base their\nchanges, which is the most suitable place for this information.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/SubmittingPatches | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex b9fd182..e810263 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -88,6 +88,10 @@ change is relevant to.\n wait until some of the dependent topics graduate to 'master', and\n rebase your work.\n \n+ - Some parts of the system have dedicated maintainers with their own\n+ repositories (see the section \"Subsystems\" below). Changes to\n+ these parts should be based on their trees.\n+\n To find the tip of a topic branch, run \"git log --first-parent\n master..pu\" and look for the merge commit. The second parent of this\n commit is the tip of the topic branch.\n@@ -279,6 +283,26 @@ You can also create your own tag or use one that's in common usage\n such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n \n ------------------------------------------------\n+Subsystems with dedicated maintainers\n+\n+Some parts of the system have dedicated maintainers with their own\n+repositories.\n+\n+ - git-gui/ comes from git-gui project, maintained by Pat Thoyts:\n+\n+ git://repo.or.cz/git-gui.git\n+\n+ - gitk-git/ comes from Paul Mackerras's gitk project:\n+\n+ git://ozlabs.org/~paulus/gitk\n+\n+ - po/ comes from the localization coordinator, Jiang Xin:\n+\n+\thttps://github.com/git-l10n/git-po/\n+\n+Patches to these parts should be based on their trees.\n+\n+------------------------------------------------\n An ideal patch flow\n \n Here is an ideal patch flow for this project the current maintainer\n-- \n1.8.1.209.gc32ab23\n"},{"id":"205814","messageId":"1357082695-29713-4-git-send-email-gitster@pobox.com","threadId":"32506","inReplyTo":"1357082695-29713-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] SubmittingPatches: remove overlong checklist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T23:24:55Z","receivedAt":"2013-01-01T23:24:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The section is no longer a concise checklist. It also talks about\nthings that are not covered in the \"Long version\" text, which means\npeople need to read both, covering more or less the same thing in\ndifferent phrasing.\n\nFold the details into the main text and remove the section.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/SubmittingPatches | 126 +++++++++++++++++-----------------------\n 1 file changed, 53 insertions(+), 73 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex e810263..a7daad3 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -1,65 +1,3 @@\n-Checklist (and a short version for the impatient):\n-\n-\tCommits:\n-\n-\t- make commits of logical units\n-\t- check for unnecessary whitespace with \"git diff --check\"\n-\t before committing\n-\t- do not check in commented out code or unneeded files\n-\t- the first line of the commit message should be a short\n-\t description (50 characters is the soft limit, see DISCUSSION\n-\t in git-commit(1)), and should skip the full stop\n-\t- it is also conventional in most cases to prefix the\n-\t first line with \"area: \" where the area is a filename\n-\t or identifier for the general area of the code being\n-\t modified, e.g.\n-\t . archive: ustar header checksum is computed unsigned\n-\t . git-cherry-pick.txt: clarify the use of revision range notation\n-\t (if in doubt which identifier to use, run \"git log --no-merges\"\n-\t on the files you are modifying to see the current conventions)\n-\t- the body should provide a meaningful commit message, which:\n-\t . explains the problem the change tries to solve, iow, what\n-\t is wrong with the current code without the change.\n-\t . justifies the way the change solves the problem, iow, why\n-\t the result with the change is better.\n-\t . alternate solutions considered but discarded, if any.\n-\t- describe changes in imperative mood, e.g. \"make xyzzy do frotz\"\n-\t instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed\n-\t xyzzy to do frotz\", as if you are giving orders to the codebase\n-\t to change its behaviour.\n-\t- try to make sure your explanation can be understood without\n-\t external resources. Instead of giving a URL to a mailing list\n-\t archive, summarize the relevant points of the discussion.\n-\t- add a \"Signed-off-by: Your Name \" line to the\n-\t commit message (or just use the option \"-s\" when committing)\n-\t to confirm that you agree to the Developer's Certificate of Origin\n-\t- make sure that you have tests for the bug you are fixing\n-\t- make sure that the test suite passes after your commit\n-\n-\tPatch:\n-\n-\t- use \"git format-patch -M\" to create the patch\n-\t- do not PGP sign your patch\n-\t- do not attach your patch, but read in the mail\n-\t body, unless you cannot teach your mailer to\n-\t leave the formatting of the patch alone.\n-\t- be careful doing cut & paste into your mailer, not to\n-\t corrupt whitespaces.\n-\t- provide additional information (which is unsuitable for\n-\t the commit message) between the \"---\" and the diffstat\n-\t- if you change, add, or remove a command line option or\n-\t make some other user interface change, the associated\n-\t documentation should be updated as well.\n-\t- if your name is not writable in ASCII, make sure that\n-\t you send off a message in the correct encoding.\n-\t- send the patch to the list (git@vger.kernel.org) and the\n-\t maintainer (gitster@pobox.com) if (and only if) the patch\n-\t is ready for inclusion. If you use git-send-email(1),\n-\t please test it first by sending email to yourself.\n-\t- see below for instructions specific to your mailer\n-\n-Long version:\n-\n Here are some guidelines for people who want to contribute their code\n to this software.\n \n@@ -119,13 +57,52 @@ change, the approach taken by the change, and if relevant how this\n differs substantially from the prior version, are all good things\n to have.\n \n+Make sure that you have tests for the bug you are fixing.\n+\n+When adding a new feature, make sure that you have new tests to show\n+the feature triggers the new behaviour when it should, and to show the\n+feature does not trigger when it shouldn't. Also make sure that the\n+test suite passes after your commit. Do not forget to update the\n+documentation to describe the updated behaviour.\n+\n Oh, another thing. I am picky about whitespaces. Make sure your\n changes do not trigger errors with the sample pre-commit hook shipped\n in templates/hooks--pre-commit. To help ensure this does not happen,\n run git diff --check on your changes before you commit.\n \n+(2) Describe your changes well.\n+\n+The first line of the commit message should be a short description (50\n+characters is the soft limit, see DISCUSSION in git-commit(1)), and\n+should skip the full stop. It is also conventional in most cases to\n+prefix the first line with \"area: \" where the area is a filename or\n+identifier for the general area of the code being modified, e.g.\n+\n+ . archive: ustar header checksum is computed unsigned\n+ . git-cherry-pick.txt: clarify the use of revision range notation\n+\n+If in doubt which identifier to use, run \"git log --no-merges\" on the\n+files you are modifying to see the current conventions.\n+\n+The body should provide a meaningful commit message, which:\n \n-(2) Generate your patch using git tools out of your commits.\n+ . explains the problem the change tries to solve, iow, what is wrong\n+ with the current code without the change.\n+\n+ . justifies the way the change solves the problem, iow, why the\n+ result with the change is better.\n+\n+ . alternate solutions considered but discarded, if any.\n+\n+Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n+instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n+to do frotz\", as if you are giving orders to the codebase to change\n+its behaviour. Try to make sure your explanation can be understood\n+without external resources. Instead of giving a URL to a mailing list\n+archive, summarize the relevant points of the discussion.\n+\n+\n+(3) Generate your patch using git tools out of your commits.\n \n git based diff tools generate unidiff which is the preferred format.\n \n@@ -133,22 +110,27 @@ You do not have to be afraid to use -M option to \"git diff\" or\n \"git format-patch\", if your patch involves file renames. The\n receiving end can handle them just fine.\n \n-Please make sure your patch does not include any extra files\n-which do not belong in a patch submission. Make sure to review\n+Please make sure your patch does not add commented out debugging code,\n+or include any extra files which do not relate to what your patch\n+is trying to achieve. Make sure to review\n your patch after generating it, to ensure accuracy. Before\n sending out, please make sure it cleanly applies to the \"master\"\n branch head. If you are preparing a work based on \"next\" branch,\n that is fine, but please mark it as such.\n \n \n-(3) Sending your patches.\n+(4) Sending your patches.\n \n People on the git mailing list need to be able to read and\n comment on the changes you are submitting. It is important for\n a developer to be able to \"quote\" your changes, using standard\n e-mail tools, so that they may comment on specific portions of\n your code. For this reason, all patches should be submitted\n-\"inline\". WARNING: Be wary of your MUAs word-wrap\n+\"inline\". If your log message (including your name on the\n+Signed-off-by line) is not writable in ASCII, make sure that\n+you send off a message in the correct encoding.\n+\n+WARNING: Be wary of your MUAs word-wrap\n corrupting your patch. Do not cut-n-paste your patch; you can\n lose tabs that way if you are not careful.\n \n@@ -200,16 +182,14 @@ patch, format it as \"multipart/signed\", not a text/plain message\n that starts with '-----BEGIN PGP SIGNED MESSAGE-----'. That is\n not a text/plain, it's something else.\n \n-Unless your patch is a very trivial and an obviously correct one,\n-first send it with \"To:\" set to the mailing list, with \"cc:\" listing\n+Send your patch with \"To:\" set to the mailing list, with \"cc:\" listing\n people who are involved in the area you are touching (the output from\n \"git blame $path\" and \"git shortlog --no-merges $path\" would help to\n identify them), to solicit comments and reviews. After the list\n reached a consensus that it is a good idea to apply the patch, re-send\n-it with \"To:\" set to the maintainer and optionally \"cc:\" the list for\n-inclusion. Do not forget to add trailers such as \"Acked-by:\",\n-\"Reviewed-by:\" and \"Tested-by:\" after your \"Signed-off-by:\" line as\n-necessary.\n+it with \"To:\" set to the maintainer and \"cc:\" the list for inclusion.\n+Do not forget to add trailers such as \"Acked-by:\", \"Reviewed-by:\" and\n+\"Tested-by:\" after your \"Signed-off-by:\" line as necessary.\n \n \n (4) Sign your work\n-- \n1.8.1.209.gc32ab23\n"},{"id":"205826","messageId":"20130102015233.GA25288@gmail.com","threadId":"32506","inReplyTo":"1357082695-29713-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories","fromName":"Jason Holden","fromEmail":"jason.k.holden.swdev@gmail.com","sentAt":"2013-01-02T01:52:34Z","receivedAt":"2013-01-02T01:52:34Z","isPatch":true,"sender":{"key":"jason.k.holden.swdev@gmail.com","avatar":null},"body":"On Tue, Jan 01, 2013 at 03:24:54PM -0800, Junio C Hamano wrote:\n> \n> ------------------------------------------------\n> +Subsystems with dedicated maintainers\n> +\n> +Some parts of the system have dedicated maintainers with their own\n> +repositories.\n> +\n> + - git-gui/ comes from git-gui project, maintained by Pat Thoyts:\n> +\n> + git://repo.or.cz/git-gui.git\n> +\n> + - gitk-git/ comes from Paul Mackerras's gitk project:\n> +\n> + git://ozlabs.org/~paulus/gitk\n> +\n> + - po/ comes from the localization coordinator, Jiang Xin:\n> +\n> +\thttps://github.com/git-l10n/git-po/\n> +\n> +Patches to these parts should be based on their trees.\n> +\n> +------------------------------------------------\n> An ideal patch flow\n> \n\nAny reason to leave out the maintainers email addresses? If we add that, all\nthe content of the MAINTAINERS file we had been discussing would then be in\nthis file.\n\nMy only other suggestion for this series might be to augment the file with \na patch submittal example(s). I found the best example to be in \ngit-send-email's man page, but maybe enumerate the example out for the \nmost common workflows. Maybe something like:\n\n-----------------------------------------------------------\nExample of sending patches for a new feature:\n\nCreate the patches:\n $ git format-patch --cover-letter -M origin/master -o outgoing/\n $ edit outgoing/0000-*\n\nTo send your completed patches for initial consideration:\n $ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com\n\nTo send your reviewed patches for inclusion:\n $ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org\n\n-Jason\n"},{"id":"205827","messageId":"7vsj6k5o1w.fsf@alter.siamese.dyndns.org","threadId":"32506","inReplyTo":"20130102015233.GA25288@gmail.com","subject":"Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-02T02:14:19Z","receivedAt":"2013-01-02T02:14:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jason Holden writes:\n\n> Any reason to leave out the maintainers email addresses?\n\nNothing particular, other than that I did not find anywhere in the\nfile that does not break the flow.\n\n> My only other suggestion for this series might be to augment the file with \n> a patch submittal example(s). I found the best example to be in \n> git-send-email's man page,...\n\nI'd feel better to avoid copying and pasting. If send-email has\ngood examples, shouldn't it be sufficient to refer to them?\n\n> -----------------------------------------------------------\n> Example of sending patches for a new feature:\n>\n> Create the patches:\n> $ git format-patch --cover-letter -M origin/master -o outgoing/\n> $ edit outgoing/0000-*\n>\n> To send your completed patches for initial consideration:\n> $ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com\n\nThis is ambiguous; it makes it look as if you are CC'ing the\nmaintainer, but for the initial round, it is likely that you are\nsending it to me only because I have been involved in the area the\npatch touches.\n\n> To send your reviewed patches for inclusion:\n> $ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org\n\nThis is fine, but we would probably want to see it CC'ed to people\nwho reviewed the initial submission, too.\n"},{"id":"205850","messageId":"20130102090803.GC9328@sigill.intra.peff.net","threadId":"32506","inReplyTo":"1357082695-29713-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/3] Update SubmittingPatches","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-02T09:08:03Z","receivedAt":"2013-01-02T09:08:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 01, 2013 at 03:24:52PM -0800, Junio C Hamano wrote:\n\n> The main thing this series wants to do is the second one, but I\n> wanted to reduce the clutter in this document while at it.\n> \n> Junio C Hamano (3):\n> SubmittingPatches: who am I and who cares?\n> SubmittingPatches: mention subsystems with dedicated repositories\n> SubmittingPatches: remove overlong checklist\n\nFWIW, all look reasonable to me. I did not pay close attention to the\nearlier discussion on MAINTAINERS, but my first thought on reading it\nwas \"why isn't this stuff just in the SubmittingPatches doc? That's\nwhere I would look for it\".\n\nI guess some people might balk at the third one, as the checklist is\nsupposed to help with a quick and easy start. But I agree with your\ncommit log message that it isn't short or quick anymore. Of course it\nhas been a long time since I have needed to read that document. Maybe\npeople who have used it more recently would have more relevant thoughts.\n\n-Peff\n"},{"id":"205870","messageId":"7vy5gb33f9.fsf@alter.siamese.dyndns.org","threadId":"32506","inReplyTo":"7vsj6k5o1w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-02T17:22:50Z","receivedAt":"2013-01-02T17:22:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jason Holden writes:\n>\n>> Any reason to leave out the maintainers email addresses?\n>\n> Nothing particular, other than that I did not find anywhere in the\n> file that does not break the flow.\n\nI've prepared this on top of the previous three. The second hunk\nthat updates \"(4) Sending your patches.\" is the logical place to\nhave this information.\n\nThe other hunks are correcting minor mistakes that are not related.\nI think I'll squash them to the patch 3/3.\n\n\n Documentation/SubmittingPatches | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex a7daad3..9e113d0 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -31,8 +31,9 @@ change is relevant to.\n these parts should be based on their trees.\n \n To find the tip of a topic branch, run \"git log --first-parent\n-master..pu\" and look for the merge commit. The second parent of this\n-commit is the tip of the topic branch.\n+master..pu\" and look for merge commits. The second parent of these\n+commits are the tips of topic branches.\n+\n \n (1) Make separate commits for logically separate changes.\n \n@@ -70,6 +71,7 @@ changes do not trigger errors with the sample pre-commit hook shipped\n in templates/hooks--pre-commit. To help ensure this does not happen,\n run git diff --check on your changes before you commit.\n \n+\n (2) Describe your changes well.\n \n The first line of the commit message should be a short description (50\n@@ -185,14 +187,20 @@ not a text/plain, it's something else.\n Send your patch with \"To:\" set to the mailing list, with \"cc:\" listing\n people who are involved in the area you are touching (the output from\n \"git blame $path\" and \"git shortlog --no-merges $path\" would help to\n-identify them), to solicit comments and reviews. After the list\n-reached a consensus that it is a good idea to apply the patch, re-send\n-it with \"To:\" set to the maintainer and \"cc:\" the list for inclusion.\n-Do not forget to add trailers such as \"Acked-by:\", \"Reviewed-by:\" and\n-\"Tested-by:\" after your \"Signed-off-by:\" line as necessary.\n+identify them), to solicit comments and reviews.\n+\n+After the list reached a consensus that it is a good idea to apply the\n+patch, re-send it with \"To:\" set to the maintainer [*1*] and \"cc:\" the\n+list [*2*] for inclusion. Do not forget to add trailers such as\n+\"Acked-by:\", \"Reviewed-by:\" and \"Tested-by:\" after your\n+\"Signed-off-by:\" line as necessary.\n+\n+ [Addresses]\n+ *1* The current maintainer: gitster@pobox.com\n+ *2* The mailing list: git@vger.kernel.org\n \n \n-(4) Sign your work\n+(5) Sign your work\n \n To improve tracking of who did what, we've borrowed the\n \"sign-off\" procedure from the Linux kernel project on patches\n@@ -304,7 +312,8 @@ suggests to the contributors:\n even get them in a \"on top of your change\" patch form.\n \n (3) Polish, refine, and re-send to the list and the people who\n- spend their time to improve your patch. Go back to step (2).\n+ spent their time to help improving your patch. Go back to\n+ step (2) until step (4) happens.\n \n (4) The list forms consensus that the last round of your patch is\n good. Send it to the list and cc the maintainer.\n"},{"id":"205975","messageId":"1357333116-6971-1-git-send-email-jason.k.holden.swdev@gmail.com","threadId":"32506","inReplyTo":"7vy5gb33f9.fsf@alter.siamese.dyndns.org","subject":"[PATCH] SubmittingPatches: Document how to request a patch review tag","fromName":"Jason Holden","fromEmail":"jason.k.holden.swdev@gmail.com","sentAt":"2013-01-04T20:58:36Z","receivedAt":"2013-01-04T20:58:36Z","isPatch":true,"sender":{"key":"jason.k.holden.swdev@gmail.com","avatar":null},"body":"Document the preferred way a developer should request to have their\nAcked-by/Tested-by/Reviewed-by tag to a patch series under discussion\n\nSigned-off-by: Jason Holden \n---\nJunio,\n I was ready to add my Reviewed-by to this patch series, but I wasn't sure if\nI should email just you the patch author (to cut down on overall list traffic)\nor both you and the list. If all reviewed-by/acked-by/tested-by traffic \nshould go via the email list I think this patch would be helpful, as I \nwasn't quite sure how wide of a distribution list to use for my \n\"Reviewed-by\" email.\n\nA very similiar question was asked previously in:\nhttp://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570\n\nThis will apply on top of your last tweak to SubmittingPatches\n\nPlease add my reviewed-by to the rest of the patches in this series.\n\n-Jason\n\n Documentation/SubmittingPatches | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex f6276ff..80001c9 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -268,6 +268,11 @@ If you like, you can put extra tags at the end:\n 4. \"Tested-by:\" is used to indicate that the person applied the patch\n and found it to have the desired effect.\n \n+If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag\n+to a patch series under discussion (after having reviewed it or tested it\n+of course!), reply to the author of the patch series, cc'ing the git mailing\n+list.\n+\n You can also create your own tag or use one that's in common usage\n such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n \n-- \n1.8.1.rc3.28.g0ab5d1f\n"},{"id":"205983","messageId":"7v4niwtyb5.fsf@alter.siamese.dyndns.org","threadId":"32506","inReplyTo":"1357333116-6971-1-git-send-email-jason.k.holden.swdev@gmail.com","subject":"Re: [PATCH] SubmittingPatches: Document how to request a patch review tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-04T21:47:58Z","receivedAt":"2013-01-04T21:47:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jason Holden writes:\n\n> A very similiar question was asked previously in:\n> http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570\n\n\"Reviewed-by\" is for those who are familiar with the part of the\nsystem being touched to say \"I reviewed this patch, it looks good\",\nand Michael indeed was involved in recent updates to the refs.c\ninfrastructure, so as he said in his message \"it looks like I should\",\nit was the right thing to do.\n\nI do not think Michael was asking if that was the standard _thing_\nto do; I think the question was if there was a standard _way_\n(perhaps a tool) to send such a \"Reviewed-by:\" line.\n\n> This will apply on top of your last tweak to SubmittingPatches\n>\n> Please add my reviewed-by to the rest of the patches in this series.\n\nI do not think you \"own\" anyting in SubmittingPatches document,\nthough; at least not yet.\n"},{"id":"206117","messageId":"50E92875.6080305@alum.mit.edu","threadId":"32506","inReplyTo":"1357333116-6971-1-git-send-email-jason.k.holden.swdev@gmail.com","subject":"Re: [PATCH] SubmittingPatches: Document how to request a patch review tag","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-06T07:32:05Z","receivedAt":"2013-01-06T07:32:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/04/2013 09:58 PM, Jason Holden wrote:\n> Document the preferred way a developer should request to have their\n> Acked-by/Tested-by/Reviewed-by tag to a patch series under discussion\n> \n> Signed-off-by: Jason Holden \n> ---\n> Junio,\n> I was ready to add my Reviewed-by to this patch series, but I wasn't sure if\n> I should email just you the patch author (to cut down on overall list traffic)\n> or both you and the list. If all reviewed-by/acked-by/tested-by traffic \n> should go via the email list I think this patch would be helpful, as I \n> wasn't quite sure how wide of a distribution list to use for my \n> \"Reviewed-by\" email.\n> \n> A very similiar question was asked previously in:\n> http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570\n\nOn 01/04/2013 10:47 PM, Junio C Hamano wrote:\n> \"Reviewed-by\" is for those who are familiar with the part of the\n> system being touched to say \"I reviewed this patch, it looks good\",\n> and Michael indeed was involved in recent updates to the refs.c\n> infrastructure, so as he said in his message \"it looks like I should\",\n> it was the right thing to do.\n>\n> I do not think Michael was asking if that was the standard _thing_\n> to do; I think the question was if there was a standard _way_\n> (perhaps a tool) to send such a \"Reviewed-by:\" line.\n\nJunio is correct; I was just asking whether there was a particular email\nconvention for adding a \"Reviewed-by:\" line. It would be nice for this\nto be mentioned in the documentation.\n\n> Documentation/SubmittingPatches | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index f6276ff..80001c9 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -268,6 +268,11 @@ If you like, you can put extra tags at the end:\n> 4. \"Tested-by:\" is used to indicate that the person applied the patch\n> and found it to have the desired effect.\n> \n> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag\n> +to a patch series under discussion (after having reviewed it or tested it\n> +of course!), reply to the author of the patch series, cc'ing the git mailing\n> +list.\n> +\n> You can also create your own tag or use one that's in common usage\n> such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n\nI don't think this is quite correct. Such emails should be\n\"reply-to-all\" people who have participated in the thread, which might\ninclude more than just the patch author and the git mailing list.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"206119","messageId":"7vtxquofbd.fsf@alter.siamese.dyndns.org","threadId":"32506","inReplyTo":"50E92875.6080305@alum.mit.edu","subject":"Re: [PATCH] SubmittingPatches: Document how to request a patch review tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T09:01:42Z","receivedAt":"2013-01-06T09:01:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 01/04/2013 10:47 PM, Junio C Hamano wrote:\n>> \"Reviewed-by\" is for those who are familiar with the part of the\n>> system being touched to say \"I reviewed this patch, it looks good\",\n>> and Michael indeed was involved in recent updates to the refs.c\n>> infrastructure, so as he said in his message \"it looks like I should\",\n>> it was the right thing to do.\n>>\n>> I do not think Michael was asking if that was the standard _thing_\n>> to do; I think the question was if there was a standard _way_\n>> (perhaps a tool) to send such a \"Reviewed-by:\" line.\n>\n> Junio is correct; I was just asking whether there was a particular email\n> convention for adding a \"Reviewed-by:\" line. It would be nice for this\n> to be mentioned in the documentation.\n\nYeah, I wasn't exactly correct in that I was talking more about\nAcked-by than Reviewed-by, but they are morally very similar and the\nsame argument applies to both.\n\nIn the particular case of your \"refs.c\" review, because you are not\njust familiar with the code, but essentially are the current owner\nof the code, Acked-by might have been even more appropriate than\nReviewed-by, by the way.\n\n>> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag\n>> +to a patch series under discussion (after having reviewed it or tested it\n>> +of course!), reply to the author of the patch series, cc'ing the git mailing\n>> +list.\n>> +\n>> You can also create your own tag or use one that's in common usage\n>> such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n>\n> I don't think this is quite correct. Such emails should be\n> \"reply-to-all\" people who have participated in the thread, which might\n> include more than just the patch author and the git mailing list.\n\nThat would be more helpful. In practice, I can pick these up either\nway, but Cc'ing everybody would be better as a common courtesy.\n\nWhen the author resubmits an already reviewed patch with these Acks\nand Reviews for final application, these reviewers should be Cc'ed\nso that they can say \"Huh? that is not the exact patch I reviewed.\nWhat is going on?\".\n\nThanks for a review.\n"}]}