On Thu, Sep 22, 2016 at 9:02 AM Andrew Wilkins <andrew.wilk...@canonical.com> wrote:
> On Thu, Sep 22, 2016 at 7:59 AM Horacio Duran <horacio.du...@canonical.com> > 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). > or s/bot/github itself, as reed said/ > On Wednesday, 21 September 2016, Reed O'Brien <reed.obr...@canonical.com> > wrote: > >> On Wed, Sep 21, 2016 at 4:43 PM, Horacio Duran < >>> horacio.du...@canonical.com> 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 < >>>> reed.obr...@canonical.com> 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 < >>>>> andrew.wilk...@canonical.com> wrote: >>>>> >>>>>> On Wed, Sep 21, 2016 at 5:53 AM Menno Smits < >>>>>> menno.sm...@canonical.com> 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 < >>>>>>> rick.hard...@canonical.com> 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 < >>>>>>>> katherine.cox-bu...@canonical.com> 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 <nate.fi...@canonical.com> 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 >>>>>>>>> > <katherine.cox-bu...@canonical.com> 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 <nate.fi...@canonical.com> 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 >>>>>>>>> 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 >>>>>>>> >>>>>>>> >>>>>>> -- >>>>>>> Juju-dev mailing list >>>>>>> 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 >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Reed O'Brien >>>>> ✉ reed.obr...@canonical.com >>>>> ✆ 415-562-6797 >>>>> >>>>> >>>>> -- >>>>> Juju-dev mailing list >>>>> Juju-dev@lists.ubuntu.com >>>>> Modify settings or unsubscribe at: >>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>>>> >>>>> >>>> >>> >>> >>> -- >>> Reed O'Brien >>> ✉ reed.obr...@canonical.com >>> ✆ 415-562-6797 >>> >>>
-- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
