On 16/09/16 08:02, Ian Booth wrote: > Another data point - in the past, when we have had PRs which touch a lot of > files (eg change the import path for a dependency), review board paginates the > diff so it's much easier to manage, whereas I've seen github actually truncate > what it displays because the diff is "too large". Hopefully this will no > longer > be an issue, or else we won't be able to review such changes in the future. This is perfect to reduce the size of our proposals to manageable :) > > On 16/09/16 07:53, Menno Smits wrote: >> Although I share some of Ian's concerns, I think the reduced moving parts, >> improved reliability, reduced maintenance, easier experience for outside >> contributors and better handling of file moves are pretty big wins. The >> rendering of diffs on Github is a whole lot nicer as well. >> >> I'm +1 for trialling the new review system on Github for a couple of weeks >> as per Andrew's suggestion. >> >> On 16 September 2016 at 05:50, Nate Finch <nate.fi...@canonical.com> wrote: >> >>> Reviewboard goes down a couple times a month, usually from lack of disk >>> space or some other BS. According to a source knowledgeable with these >>> matters, the charm was rushed out, and the agent for that machine is down >>> anyway, so we're kinda just waiting for the other shoe to drop. >>> >>> As for the process things that Ian mentioned, most of those can be >>> addressed with a sprinkling of convention. Marking things as issues could >>> just be adding :x: to the first line (github even pops up suggestions and >>> auto-completes), thusly: >>> >>> [image: :x:]This will cause a race condition >>> >>> And if you want to indicate you're dropping a suggestion, you can use :-1: >>> which gives you a thumbs down: >>> >>> [image: :-1:] I ran the race detector and it's fine. >>> >>> It won't give you the cumulative "what's left to fix" at the top of the >>> page, like reviewboard... but for me, I never directly read that, anyway, >>> just used it to see if there were zero or non-zero comments left. >>> >>> As for the inline comments in the code - there's a checkbox to hide them >>> all. It's not quite as convenient as the gutter indicators per-comment, >>> but it's sufficient, I think. >>> >>> On Wed, Sep 14, 2016 at 6:43 PM Ian Booth <ian.bo...@canonical.com> wrote: >>> >>>> >>>> On 15/09/16 08:22, Rick Harding wrote: >>>>> I think that the issue is that someone has to maintain the RB and the >>>>> cost/time spent on that does not seem commensurate with the bonus >>>> features >>>>> in my experience. >>>>> >>>> The maintenance is not that great. We have SSO using github credentials so >>>> there's really no day to day works IIANM. As a team, we do many, many >>>> reviews >>>> per day, and the features I outlined are significant and things I (and I >>>> assume >>>> others) rely on. Don't under estimate the value in knowing why a comment >>>> was >>>> rejected and being able to properly track that. Or having review comments >>>> collapsed as a gutter indicated so you can browse the code and still know >>>> that >>>> there's a comment there. With github, you can hide the comments but >>>> there's no >>>> gutter indicator. All these things add up to a lot. >>>> >>>> >>>>> On Wed, Sep 14, 2016 at 6:13 PM Ian Booth <ian.bo...@canonical.com> >>>> wrote: >>>>>> One thing review board does better is use gutter indicators so as not >>>> to >>>>>> interrupt the flow of reading the code with huge comment blocks. It >>>> also >>>>>> seems >>>>>> much better at allowing previous commits with comments to be viewed in >>>>>> their >>>>>> entirety. And it allows the reviewer to differentiate between issues >>>> and >>>>>> comments (ie fix this vs take note of this), plus it allows the notion >>>> of >>>>>> marking stuff as fixed vs dropped, with a reason for dropping if >>>> needed. >>>>>> So the >>>>>> github improvements are nice but there's still a large and significant >>>> gap >>>>>> that >>>>>> is yet to be filled. I for one would miss all the features reviewboard >>>>>> offers. >>>>>> Unless there's a way of doing the same thing in github that I'm not >>>> aware >>>>>> of. >>>>>> >>>>>> On 15/09/16 07:22, Tim Penhey wrote: >>>>>>> I'm +1 if we can remove the extra tools and we don't get email per >>>>>> comment. >>>>>>> On 15/09/16 08:03, Nate Finch wrote: >>>>>>>> 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
