On Thu, Sep 22, 2016 at 7:59 AM Horacio Duran <[email protected]> wrote:
> 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 > 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). > On Wednesday, 21 September 2016, Reed O'Brien <[email protected]> > wrote: > >> On Wed, Sep 21, 2016 at 4:43 PM, Horacio Duran < >> [email protected]> wrote: >> >>> I disagree, once the discussion is over and the merge is ready, an >>> amend/squash is in order to avoid useless tree nodes. >>> >> 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. >> >> Obviously we need to use gerrit. /me ducks... >> >> >>> >>> On Wed, Sep 21, 2016 at 8:41 PM, Reed O'Brien <[email protected] >>> > wrote: >>> >>>> Two things I've noticed today while doing OCR duties are: >>>> 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. >>>> >>> Indeed, this isn't great for when you're OCR. > 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. >>>> >>>> >>>> On Wed, Sep 21, 2016 at 4:37 AM, Andrew Wilkins < >>>> [email protected]> wrote: >>>> >>>>> On Wed, Sep 21, 2016 at 5:53 AM Menno Smits <[email protected]> >>>>> wrote: >>>>> >>>>>> 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. >>>>>> >>>>> >>>>> 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. >>>>> >>>>> There are pros and cons to each; neither is perfect. You can find the >>>>> main points of discussion in the tech board agenda. >>>>> >>>>> Please give it a shot and provide your criticisms so we decide on the >>>>> best path forward at the end of the trial. >>>>> >>>>> Cheers, >>>>> Andrew >>>>> >>>>> 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 >>>>>> >>>>>> On 21 September 2016 at 05:54, Rick Harding < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> 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. >>>>>>> >>>>>>> On Tue, Sep 20, 2016 at 1:52 PM Katherine Cox-Buday < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>>> 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". >>>>>>>> >>>>>>>> -- >>>>>>>> Katherine >>>>>>>> >>>>>>>> Nate Finch <[email protected]> writes: >>>>>>>> >>>>>>>> > Can we try reviews on github for a couple weeks? Seems like we'll >>>>>>>> > never know if it's sufficient if we don't try it. And there's no >>>>>>>> setup >>>>>>>> > cost, which is nice. >>>>>>>> > >>>>>>>> > On Tue, Sep 20, 2016 at 12:44 PM Katherine Cox-Buday >>>>>>>> > <[email protected]> wrote: >>>>>>>> > >>>>>>>> > I see quite a few PRs that are being reviewed in GitHub and >>>>>>>> not >>>>>>>> > ReviewBoard. I really don't care where we do them, but can we >>>>>>>> > please pick a direction and move forward? And until then, can >>>>>>>> we >>>>>>>> > stick to our previous decision and use RB? With people using >>>>>>>> both >>>>>>>> > it's much more difficult to tell what's been reviewed and what >>>>>>>> > hasn't. >>>>>>>> > >>>>>>>> > -- >>>>>>>> > Katherine >>>>>>>> > >>>>>>>> > Nate Finch <[email protected]> writes: >>>>>>>> > >>>>>>>> > > In case you missed it, Github rolled out a new review >>>>>>>> process. >>>>>>>> > It >>>>>>>> > > basically works just like reviewboard does, where you start >>>>>>>> a >>>>>>>> > review, >>>>>>>> > > batch up comments, then post the review as a whole, so you >>>>>>>> don't >>>>>>>> > just >>>>>>>> > > write a bunch of disconnected comments (and get one email >>>>>>>> per >>>>>>>> > review, >>>>>>>> > > not per comment). The only features reviewboard has is the >>>>>>>> edge >>>>>>>> > case >>>>>>>> > > stuff that we rarely use: like using rbt to post a review >>>>>>>> from a >>>>>>>> > > random diff that is not connected directly to a github PR. I >>>>>>>> > think >>>>>>>> > > that is easy enough to give up in order to get the benefit >>>>>>>> of >>>>>>>> > not >>>>>>>> > > needing an entirely separate system to handle reviews. >>>>>>>> > > >>>>>>>> > > I made a little test review on one PR here, and the UX was >>>>>>>> > almost >>>>>>>> > > exactly like working in reviewboard: >>>>>>>> > > https://github.com/juju/juju/pull/6234 >>>>>>>> > > >>>>>>>> > > There may be important edge cases I'm missing, but I think >>>>>>>> it's >>>>>>>> > worth >>>>>>>> > > looking into. >>>>>>>> > > >>>>>>>> > > -Nate >>>>>>>> >>>>>>>> -- >>>>>>>> Juju-dev mailing list >>>>>>>> [email protected] >>>>>>>> Modify settings or unsubscribe at: >>>>>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Juju-dev mailing list >>>>>>> [email protected] >>>>>>> Modify settings or unsubscribe at: >>>>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>>>>> >>>>>>> >>>>>> -- >>>>>> Juju-dev mailing list >>>>>> [email protected] >>>>>> Modify settings or unsubscribe at: >>>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>>>> >>>>> >>>>> -- >>>>> Juju-dev mailing list >>>>> [email protected] >>>>> Modify settings or unsubscribe at: >>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>>> >>>>> >>>> >>>> >>>> -- >>>> Reed O'Brien >>>> ✉ [email protected] >>>> ✆ 415-562-6797 >>>> >>>> >>>> -- >>>> Juju-dev mailing list >>>> [email protected] >>>> Modify settings or unsubscribe at: >>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>> >>>> >>> >> >> >> -- >> Reed O'Brien >> ✉ [email protected] >> ✆ 415-562-6797 >> >>
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
