End Of Review marker

[Jesse Meek]

In my experience submitting/reviewing is an ongoing/open discussion 
(augmented by IRC, email and hangouts) on the code which ends with an 
LGTM or some other agreement by all involved in the review/discussion. I 
think embracing and supporting this, somewhat messy, process will yield 
more productivity than over mechanising the process.

I may be proposing particularly messy PRs but in my experience I've 
always had follow up PRs and comments which render any marker (other 
than LGTM) arbitrary.

On 13/06/14 01:23, Matthew Williams wrote:
> +1 Rick.
>
> My opinion is it's just an issue of manners. After filling in comments 
> inline let them know that you're done so they can start working on it. 
> Likewise if you start doing a review and have some interruption and 
> are unable to finish it let them know that there might be more to come.
>
> It's polite
>
> Matty
>
>
> On Thu, Jun 12, 2014 at 12:48 PM, Richard Harding 
> <rick.harding at canonical.com <mailto:rick.harding at canonical.com>> wrote:
>
>     We try to put a main comment on the review. Having inline comments
>     does not
>     complete the review. So after going through the diff, we go back
>     to the
>     main pull request and leave a comment there.
>
>     "This looks good but I had a few concerns about how it's tested.
>     Please
>     don't forget to update the docs as well. Looking forward to this
>     change."
>
>     It kind of wraps it up.
>
>     The other thing we often do is ping in irc that the review is :+1: or
>     'comments sent'.
>
>     https://github.com/juju/juju-gui/pull/328 is kind of an example
>     conversation.
>
>     Rick
>
>     On Thu, 12 Jun 2014, Ian Booth wrote:
>
>     > It's also the same when you are responding to review comments.
>     You want to mark
>     > them all as Done (or whatever) and have those go out in a batch
>     to let the
>     > reviewer know they can come back and +1.
>     >
>     > Surely we're not the only people annoyed by this? I wonder what
>     more experienced
>     > github users do. Or maybe people know that github sucks for code
>     reviews and use
>     > gerrit or something else?
>     >
>     > On 12/06/14 20:38, Horacio Duran wrote:
>     > > Hey, I don't know if this bugs everyone or just me but it
>     happens very
>     > > often that I am working while people are reviewing my code on
>     gh. While
>     > > people is reviewing and commenting on the code I keep getting
>     mails and the
>     > > diff page from the pr keeps changing. To know when its all
>     done and I can
>     > > finally try to answer/fix all the comments I usually wait
>     until my phone
>     > > stops ringing madly with mails but I think that we could do
>     better. At the
>     > > end of the diff page there is a comment box where you can add
>     comments
>     > > (where you usually add your $$merge$$ or LGTM) We could add
>     something
>     > > there, like "Done" just to let the author know we are done
>     with the review
>     > > and not just reading a big confusing chunk of code.
>     > > What do you people think?
>
>     --
>     Juju-dev mailing list
>     Juju-dev at lists.ubuntu.com <mailto:Juju-dev at lists.ubuntu.com>
>     Modify settings or unsubscribe at:
>     https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140613/9407d6f2/attachment.html>