<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 19, 2014 at 12:33 PM, Dan Prince <span dir="ltr"><<a href="mailto:dprince@redhat.com" target="_blank">dprince@redhat.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Perhaps one of the lesser know Gerrit "features" is the ability to overwrite someone else's patchset/review with a new revision. This can be a handy thing for collaboration, or perhaps to make minor edits (spelling fixes for example) to help expedite the review process. Generally I think things are fine and friendly on this front. There are a couple side effect behaviors that can occur.<br>


<br></blockquote><div><br></div><div>o/ I do this regularly to help authors land their intended changes (hopefully with less frustration than they would otherwise experience). Most frequently, if the only thing holding me back from a +1 / +2 is a few nits, I'll leave some brief review feedback on the current patchset, and submit a subsequent patchset with the nits fixed, and leave a +1 / +2.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Things like: Changing the author or adding yourself as a co-author. Changing the original author should almost never happen (I'm not sure that it has). Adding yourself as a co-author is less of an issue, but is also somewhat questionable if for example all you've done is re-worded something or fixed a spelling issue. So long as the original author is in the know here I think it is probably fine to add yourself as a co-author. But making more meaningful changes, even to a commit message should be checked ahead of time so as not to disrupt the intent of the original authors patch IMO.</blockquote>

<div><br></div><div>+1 absolutely agree with these guidelines. Continuing the above, when I want to make more meaningful changes, I either A) suggest a pastebin's diff to the author, or B) go ahead and make the changes but ask that the original author review the latest patchset themselves and express a +1 to acknowledge the result.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Leaving clear Gerrit feedback on the most recent patchset/commit with a -1 should do just fine in most cases if you would like a meaningful change and aren't closely collaborating (already) on the fix...<br>


<br>
It has also come to my attention that co-authoring a patch steals the Launchpad ticket. I believe this is something that we should watch closely (and perhaps fix if we can).<br></blockquote><div><br></div><div>+1 I used to make a habit of jumping to the bug and assigning the bug "back," but depending on your definition of "steal" (what does it actually impact?), I'm not sure it's worth the effort? Regardless, I'd appreciate it if the LP bot implementing this behavior used the Author (which as you alluded, must be manually revised, e.g. `git commit --amend --author`) on the commit rather than the Committer.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Not trying to point the finger at anyone specifically here. I've probably been guilty of clobbering violations and/or accidental ticket stealing myself. We just need to be careful with these more advanced collaborative coding workflows so as not to step on each others toes.<br>

</blockquote><div><br></div><div>Thanks for bringing this up! Gerrit provides for some powerful workflows and I'd love it if the community was more comfortable taking advantage of them.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<br>
Dan<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><br></div></div>