{"thread":{"id":"15857","subject":"Adding Reviewed-by/Tested-by tags to other peoples commits","startedAt":"2008-10-11T06:37:04Z","lastAt":"2008-10-11T21:18:43Z","messageCount":8,"participants":["Alex Bennee","Jeff King","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"92784","messageId":"b2cdc9f30810102337q13432bepa957acaace9ddc5d@mail.gmail.com","threadId":"15857","inReplyTo":null,"subject":"Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Alex Bennee","fromEmail":"kernel-hacker@bennee.com","sentAt":"2008-10-11T06:37:04Z","receivedAt":"2008-10-11T06:37:04Z","isPatch":false,"sender":{"key":"kernel-hacker@bennee.com","avatar":null},"body":"HI,\n\nI've just tested/reviewed a patch of someone elses and I want to\nforward it on the appropriate mailing list. I gather for Linux you\njust add the appropriate tags to the commit. Does git offer a shortcut\nfor doing this or do you have to do a reset HEAD^ and re-commit with a\ncopy&pasted and modified commit message?\n\nOr does everyone have their own magic scripts for tagging commits\nduring reviews?\n\n-- \nAlex, homepage: http://www.bennee.com/~alex/\n"},{"id":"92785","messageId":"20081011074604.GA23883@sigill.intra.peff.net","threadId":"15857","inReplyTo":"b2cdc9f30810102337q13432bepa957acaace9ddc5d@mail.gmail.com","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-11T07:46:04Z","receivedAt":"2008-10-11T07:46:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:\n\n> I've just tested/reviewed a patch of someone elses and I want to\n> forward it on the appropriate mailing list. I gather for Linux you\n> just add the appropriate tags to the commit. Does git offer a shortcut\n> for doing this or do you have to do a reset HEAD^ and re-commit with a\n> copy&pasted and modified commit message?\n\nTry \"git commit --amend\" to edit the commit message. There's no\nautomatic way of adding acked-by or tested-by tags with git; most people\nwho do those things often would probably configure their editor to make\nit easier.\n\n-Peff\n"},{"id":"92787","messageId":"alpine.DEB.1.00.0810111239590.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15857","inReplyTo":"b2cdc9f30810102337q13432bepa957acaace9ddc5d@mail.gmail.com","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-11T10:42:08Z","receivedAt":"2008-10-11T10:42:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 11 Oct 2008, Alex Bennee wrote:\n\n> I've just tested/reviewed a patch of someone elses and I want to forward \n> it on the appropriate mailing list. I gather for Linux you just add the \n> appropriate tags to the commit. Does git offer a shortcut for doing this \n> or do you have to do a reset HEAD^ and re-commit with a copy&pasted and \n> modified commit message?\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/75250/focus=76304\n\nIn the end, nothing happened, but I could see that you might want to push \nfor this patch. However, you'll have to replace the strbuf_initf() by \nthe ugly strbuf_init() && strbuf_addf() sequence, because strbuf_initf() \nnever made it into git.git.\n\nCiao,\nDscho\n"},{"id":"92788","messageId":"alpine.DEB.1.00.0810111254240.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15857","inReplyTo":"20081011074604.GA23883@sigill.intra.peff.net","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-11T10:58:57Z","receivedAt":"2008-10-11T10:58:57Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 11 Oct 2008, Jeff King wrote:\n\n> On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:\n> \n> > I've just tested/reviewed a patch of someone elses and I want to \n> > forward it on the appropriate mailing list. I gather for Linux you \n> > just add the appropriate tags to the commit. Does git offer a shortcut \n> > for doing this or do you have to do a reset HEAD^ and re-commit with a \n> > copy&pasted and modified commit message?\n> \n> Try \"git commit --amend\" to edit the commit message. There's no \n> automatic way of adding acked-by or tested-by tags with git; most people \n> who do those things often would probably configure their editor to make \n> it easier.\n\nOf course, you could also automate it with a script:\n\n-- snip --\nGIT_EDITOR='sh -c \"perl -pi.bak -e s/pick/edit/ \\\"$1\\\"\"' \\\ngit rebase -i HEAD~2\nwhile test -d .git/rebase-merge\ndo\n GIT_EDITOR='sh -c \"echo \\\"Reviewed-by: Mini Me \\\" >> \\\"$1\\\"\"' \\\n git commit --amend &&\n git rebase --continue ||\n break\ndone\n-- snap --\n\n(Of course you would make HEAD~2 a parameter...)\n\nBTW what happened with the \"amend\" patches to rebase -i? AFAIR Peff or \nMadCoder (or was it j6t?) had patches to teach rebase -i the \"amend\" \ncommand, which would not fall back to the command line just to fix a typo \nin a commit message... I would like to have that.\n\nHth,\nDscho\n"},{"id":"92789","messageId":"7v4p3jl6a9.fsf@gitster.siamese.dyndns.org","threadId":"15857","inReplyTo":"20081011074604.GA23883@sigill.intra.peff.net","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-11T11:28:46Z","receivedAt":"2008-10-11T11:28:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:\n>\n>> I've just tested/reviewed a patch of someone elses and I want to\n>> forward it on the appropriate mailing list. I gather for Linux you\n>> just add the appropriate tags to the commit. Does git offer a shortcut\n>> for doing this or do you have to do a reset HEAD^ and re-commit with a\n>> copy&pasted and modified commit message?\n>\n> Try \"git commit --amend\" to edit the commit message. There's no\n> automatic way of adding acked-by or tested-by tags with git; most people\n> who do those things often would probably configure their editor to make\n> it easier.\n\nThese Distimmed-by: fields are cumulative in nature, so once recorded in a\ncommit log message you cannot add new ones without amending, which means\nyou cannot exchange such commits by pushing or pulling but by forwarding\npatches via e-mails. I thought people added these in their MUAs. IOW, I\nshare your \"would probably\" with s/editor/MUA/.\n"},{"id":"92795","messageId":"7vfxn3jqt9.fsf@gitster.siamese.dyndns.org","threadId":"15857","inReplyTo":"alpine.DEB.1.00.0810111239590.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-11T11:48:18Z","receivedAt":"2008-10-11T11:48:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Sat, 11 Oct 2008, Alex Bennee wrote:\n>\n>> I've just tested/reviewed a patch of someone elses and I want to forward \n>> it on the appropriate mailing list. I gather for Linux you just add the \n>> appropriate tags to the commit. Does git offer a shortcut for doing this \n>> or do you have to do a reset HEAD^ and re-commit with a copy&pasted and \n>> modified commit message?\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/75250/focus=76304\n>\n> In the end, nothing happened, but I could see that you might want to push \n> for this patch.\n\nThe fact a particular change has been reviewed is an attribute of a\ncommit, and by recording the fact once (perhaps when you commit for the\nfirst time, or if your workflow is \"commit blindly, then review, and then\namend\" then when you amend that commit), the commit object will remember\nthat fact.\n\nThe patch you quoted adds Reviewed-by: at format-patch time, but I suspect\nthat is a wrong approach. You have to remember and recall which ones you\nreviewed (and which ones you didn't) when you run format-patch. People\nwho commit and immediately format-patch to send, or people who do not\nreview until immediately before format-patch to send, would not realize\nthe downside of the approach, but when your work style is to perform\ncommit/review and e-mail communication in separate phases, it matters.\n\nThis is a bit tangent, but perhaps rebase needs a hook so that users can\nstrip certain tags automatically from the commit log messages (e.g.\nthings like Reviewd-by: and Tested-by: become less trustworthy when you\nrebase; S-o-b: becomes somewhat less trustworthy when you \"edit\" in\nrebase-i; etc).\n"},{"id":"92798","messageId":"alpine.DEB.1.00.0810111457300.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15857","inReplyTo":"7vfxn3jqt9.fsf@gitster.siamese.dyndns.org","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-11T13:06:48Z","receivedAt":"2008-10-11T13:06:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 11 Oct 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Sat, 11 Oct 2008, Alex Bennee wrote:\n> >\n> >> I've just tested/reviewed a patch of someone elses and I want to \n> >> forward it on the appropriate mailing list. I gather for Linux you \n> >> just add the appropriate tags to the commit. Does git offer a \n> >> shortcut for doing this or do you have to do a reset HEAD^ and \n> >> re-commit with a copy&pasted and modified commit message?\n> >\n> > http://thread.gmane.org/gmane.comp.version-control.git/75250/focus=76304\n> >\n> > In the end, nothing happened, but I could see that you might want to \n> > push for this patch.\n> \n> The fact a particular change has been reviewed is an attribute of a \n> commit, and by recording the fact once (perhaps when you commit for the \n> first time, or if your workflow is \"commit blindly, then review, and \n> then amend\" then when you amend that commit), the commit object will \n> remember that fact.\n\nIf that was the goal, you would _have_ to add commit notes. Because \notherwise you would have to pretend to have changed the commit, which you \ndid not at all.\n\n> The patch you quoted adds Reviewed-by: at format-patch time, but I \n> suspect that is a wrong approach.\n\nColor me puzzled. You said in another mail that you think this is the \ntask for the MUA. When I send patches (even forwarded ones), I use \nformat-patch just before pasting into the MUA (I do not trust send-email). \nAnd that's where Git can kick in: format-patch. Not the MUA.\n\nSo technically, I understood that the format-patch approach is exactly the \nsame as you were proposing, only that you do not ask the MUA to do Git's \njob.\n\nUnless, of course, you are talking about the reviewing style where the \npatch does not leave the MUA until it is to be forwarded.\n\n> You have to remember and recall which ones you reviewed (and which ones \n> you didn't) when you run format-patch.\n\nDon't you have to do that anyway? I do not see how giving format-patch a \nnew option --reviewed-by would change the equation in any way.\n\n> This is a bit tangent, but perhaps rebase needs a hook so that users can \n> strip certain tags automatically from the commit log messages (e.g. \n> things like Reviewd-by: and Tested-by: become less trustworthy when you \n> rebase; S-o-b: becomes somewhat less trustworthy when you \"edit\" in \n> rebase-i; etc).\n\nMaybe. I am not really convinced of the S-o-b. You kept stressing that \nthe SOB is not about validity, but a statement that the patch is \nintellectually proper or some such (IOW it means something like \"Darl, \nforget it\"). And the point of origin does not change, even if you rebase \nthe commit.\n\nCiao,\nDscho\n"},{"id":"92808","messageId":"7v7i8ekez0.fsf@gitster.siamese.dyndns.org","threadId":"15857","inReplyTo":"alpine.DEB.1.00.0810111457300.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: Adding Reviewed-by/Tested-by tags to other peoples commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-11T21:18:43Z","receivedAt":"2008-10-11T21:18:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Color me puzzled. You said in another mail that you think this is the \n> task for the MUA.\n\nNot really. I said that I think people usually do this in MUA with the\ncurrent system. I did not mean to say that I think such a partition of\njobs between commit and MUA is ideal.\n\n>> This is a bit tangent, but perhaps rebase needs a hook so that users can \n>> strip certain tags automatically from the commit log messages (e.g. \n>> things like Reviewd-by: and Tested-by: become less trustworthy when you \n>> rebase; S-o-b: becomes somewhat less trustworthy when you \"edit\" in \n>> rebase-i; etc).\n>\n> Maybe. I am not really convinced of the S-o-b. You kept stressing that \n> the SOB is not about validity, but a statement that the patch is \n> intellectually proper or some such (IOW it means something like \"Darl, \n> forget it\"). And the point of origin does not change, even if you rebase \n> the commit.\n\nThe \"somewhat less trustworthy\" kicks in when you \"edit\" in rebase-i if\nyou change the tree that gets recorded. You are right that it is\nirrelevant if you ran rebase-i to only edit the commit log message.\n"}]}