<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Sep 22, 2016 at 7:59 AM Horacio Duran <<a href="mailto:horacio.duran@canonical.com">horacio.duran@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Well I like to do my squashing because I tend to have some meaningless commit messages in the middle, I don't think project history cares about me going to the supermarket, we could make it use the gh PR message though<br></blockquote><div><br></div><div>I usually rebase and force-push after addressing review comments too. If we stick with GitHub reviews, the squashing probably should be done just before merging (or by the bot).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wednesday, 21 September 2016, Reed O'Brien <<a href="mailto:reed.obrien@canonical.com" target="_blank">reed.obrien@canonical.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Sep 21, 2016 at 4:43 PM, Horacio Duran <span dir="ltr"><<a>horacio.duran@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I disagree, once the discussion is over and the merge is ready, an amend/squash is in order to avoid useless tree nodes.</div></blockquote><div>You make a good point and I agree with you, but I think the landing bot should squash the commits at merge. I suppose if GH improves the UI around comments, reviews, and commits it would be less surprising. I can also live with it once I know how to recognized it happened TTW. </div><div><br></div><div>Obviously we need to use gerrit. /me ducks...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 21, 2016 at 8:41 PM, Reed O'Brien <span dir="ltr"><<a>reed.obrien@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Two things I've noticed today while doing OCR duties are:<div>1. There's no way to tell if a PR has a review when looking at the list of open PRs. I may be missing something or it may be an oversight on the part of GH and will likely be remedied soon. When there are comments it shows little comment clouds and a count, but not if there's only a review nothing shows there for me.</div></div></blockquote></div></div></div></div></blockquote></div></div></div></blockquote></blockquote><div><br></div><div>Indeed, this isn't great for when you're OCR.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>2. When someone amends a commit and force pushes, the review remains but isn't attached to a commit any longer. You can see that there was an update because the commit appears later in the timeline than the review on the PR details view. Personally, I think that once you make a PR and involve someone else we shouldn't rewrite history anymore.</div><div><br></div></div><div class="gmail_extra"><div><div><br><div class="gmail_quote">On Wed, Sep 21, 2016 at 4:37 AM, Andrew Wilkins <span dir="ltr"><<a>andrew.wilkins@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div dir="ltr">On Wed, Sep 21, 2016 at 5:53 AM Menno Smits <<a>menno.smits@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div style="font-family:verdana,sans-serif">Some of us probably got a little excited (me included). There should be discussion and a clear announcement before we make a signigicant change to our process. The tech board meeting is today/tonight so we'll discuss it there as per Rick's email. Please contribute to this thread if you haven't already and have strong opinions either way on the topic.</div></div></blockquote><div><br></div></span><div>We discussed Github reviews vs. Reviewboard at the tech board meeting today, and we all agreed that we should go ahead with a trial for 2 weeks.</div><div><br></div><div>There are pros and cons to each; neither is perfect. You can find the main points of discussion in the tech board agenda.</div><div><br></div><div>Please give it a shot and provide your criticisms so we decide on the best path forward at the end of the trial.</div><div><br></div><div>Cheers,</div><div>Andrew</div><div><div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div style="font-family:verdana,sans-serif">Interestingly our Github/RB integration seems to have broken a little since Github made these changes. The links to Reviewboard on pull requests aren't getting inserted any more. If we decide to stay with RB </div></div><div class="gmail_extra"><br><div class="gmail_quote">On 21 September 2016 at 05:54, Rick Harding <span dir="ltr"><<a>rick.harding@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I spoke with Alexis today about this and it's on her list to check with her folks on this. The tech board has been tasked with he decision, so please feel free to shoot a copy of your opinions their way. As you say, on the one hand it's a big impact on the team, but it's also a standard developer practice that not everyone will agree with so I'm sure the tech board is a good solution to limiting the amount of bike-shedding and to have some multi-mind consensus. <br></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 20, 2016 at 1:52 PM Katherine Cox-Buday <<a>katherine.cox-buday@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Seems like a good thing to do would be to ensure the tech board doesn't have any objections and then put it to a vote since it's more a property of the team and not the codebase.<br> <br> I just want some consistency until a decision is made. E.g. "we will be trying out GitHub reviews for the next two weeks; all reviews should be done on there".<br> <br> --<br> Katherine<br> <br> Nate Finch <<a>nate.finch@canonical.com</a>> writes:<br> <br> > Can we try reviews on github for a couple weeks? Seems like we'll<br> > never know if it's sufficient if we don't try it. And there's no setup<br> > cost, which is nice.<br> ><br> > On Tue, Sep 20, 2016 at 12:44 PM Katherine Cox-Buday<br> > <<a>katherine.cox-buday@canonical.com</a>> wrote:<br> ><br> > I see quite a few PRs that are being reviewed in GitHub and not<br> > ReviewBoard. I really don't care where we do them, but can we<br> > please pick a direction and move forward? And until then, can we<br> > stick to our previous decision and use RB? With people using both<br> > it's much more difficult to tell what's been reviewed and what<br> > hasn't.<br> ><br> > --<br> > Katherine<br> ><br> > Nate Finch <<a>nate.finch@canonical.com</a>> writes:<br> ><br> > > In case you missed it, Github rolled out a new review process.<br> > It<br> > > basically works just like reviewboard does, where you start a<br> > review,<br> > > batch up comments, then post the review as a whole, so you don't<br> > just<br> > > write a bunch of disconnected comments (and get one email per<br> > review,<br> > > not per comment). The only features reviewboard has is the edge<br> > case<br> > > stuff that we rarely use: like using rbt to post a review from a<br> > > random diff that is not connected directly to a github PR. I<br> > think<br> > > that is easy enough to give up in order to get the benefit of<br> > not<br> > > needing an entirely separate system to handle reviews.<br> > ><br> > > I made a little test review on one PR here, and the UX was<br> > almost<br> > > exactly like working in reviewboard:<br> > > <a href="https://github.com/juju/juju/pull/6234" rel="noreferrer" target="_blank">https://github.com/juju/juju/pull/6234</a><br> > ><br> > > There may be important edge cases I'm missing, but I think it's<br> > worth<br> > > looking into.<br> > ><br> > > -Nate<br> <br> --<br> Juju-dev mailing list<br> <a>Juju-dev@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br> </blockquote></div> </div></div><br>--<br> Juju-dev mailing list<br> <a>Juju-dev@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br> <br></blockquote></div><br></div> --<br> Juju-dev mailing list<br> <a>Juju-dev@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br> </blockquote></div></div></div></div> <br>--<br> Juju-dev mailing list<br> <a>Juju-dev@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br> <br></blockquote></div><br><br clear="all"><div><br></div>-- <br></div></div><span><div data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr">Reed O'Brien <div><div><span style="font-size:12.8px">✉ </span><a>reed.obrien@canonical.com</a></div><div style="font-size:12.8px">✆ <span title="Call with Google Voice"><a href="tel:415-562-6797" value="+14155626797" target="_blank">415-562-6797</a></span></div></div><div style="font-size:12.8px"><span title="Call with Google Voice"><br></span></div></div></div></div></div> </span></div> <br>--<br> Juju-dev mailing list<br> <a>Juju-dev@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br> <br></blockquote></div><br></div> </div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr">Reed O'Brien <div><div><span style="font-size:12.8px">✉ </span><a>reed.obrien@canonical.com</a></div><div style="font-size:12.8px">✆ <span title="Call with Google Voice">415-562-6797</span></div></div><div style="font-size:12.8px"><span title="Call with Google Voice"><br></span></div></div></div></div></div> </div></div> </blockquote> </blockquote></div></div>