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 Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
