Re: Adding Reviewed-by/Tested-by tags to other peoples commits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sat, 11 Oct 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> 
> > On Sat, 11 Oct 2008, Alex Bennee wrote:
> >
> >> I've just tested/reviewed a patch of someone elses and I want to 
> >> forward it on the appropriate mailing list. I gather for Linux you 
> >> just add the appropriate tags to the commit. Does git offer a 
> >> shortcut for doing this or do you have to do a reset HEAD^ and 
> >> re-commit with a copy&pasted and modified commit message?
> >
> > http://thread.gmane.org/gmane.comp.version-control.git/75250/focus=76304
> >
> > In the end, nothing happened, but I could see that you might want to 
> > push for this patch.
> 
> The fact a particular change has been reviewed is an attribute of a 
> commit, and by recording the fact once (perhaps when you commit for the 
> first time, or if your workflow is "commit blindly, then review, and 
> then amend" then when you amend that commit), the commit object will 
> remember that fact.

If that was the goal, you would _have_ to add commit notes.  Because 
otherwise you would have to pretend to have changed the commit, which you 
did not at all.

> The patch you quoted adds Reviewed-by: at format-patch time, but I 
> suspect that is a wrong approach.

Color me puzzled.  You said in another mail that you think this is the 
task for the MUA.  When I send patches (even forwarded ones), I use 
format-patch just before pasting into the MUA (I do not trust send-email).  
And that's where Git can kick in: format-patch.  Not the MUA.

So technically, I understood that the format-patch approach is exactly the 
same as you were proposing, only that you do not ask the MUA to do Git's 
job.

Unless, of course, you are talking about the reviewing style where the 
patch does not leave the MUA until it is to be forwarded.

> You have to remember and recall which ones you reviewed (and which ones 
> you didn't) when you run format-patch.

Don't you have to do that anyway?  I do not see how giving format-patch a 
new option --reviewed-by would change the equation in any way.

> This is a bit tangent, but perhaps rebase needs a hook so that users can 
> strip certain tags automatically from the commit log messages (e.g. 
> things like Reviewd-by: and Tested-by: become less trustworthy when you 
> rebase; S-o-b: becomes somewhat less trustworthy when you "edit" in 
> rebase-i; etc).

Maybe.  I am not really convinced of the S-o-b.  You kept stressing that 
the SOB is not about validity, but a statement that the patch is 
intellectually proper or some such (IOW it means something like "Darl, 
forget it").  And the point of origin does not change, even if you rebase 
the commit.

Ciao,
Dscho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux