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.hard...@canonical.com <mailto:rick.hard...@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@lists.ubuntu.com <mailto:Juju-dev@lists.ubuntu.com>
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
