End Of Review marker

[Matthew Williams]

+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> 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
> 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/20140612/f674ca85/attachment.html>