Re: Reviews on Github

2016-09-21 Thread Andrew Wilkins
On Thu, Sep 22, 2016 at 9:02 AM Andrew Wilkins 
wrote:

> On Thu, Sep 22, 2016 at 7:59 AM Horacio Duran 
> 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 
> 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  writes:
>
> > Can we try reviews on github for a couple weeks? Seems like we'll
> > never know if it's suff

Re: Reviews on Github

2016-09-21 Thread Andrew Wilkins
On Thu, Sep 22, 2016 at 7:59 AM Horacio Duran 
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 
> 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 >> > 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 
> 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  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
 >  wrote:
 >
 > I see quite a few PRs that are being reviewed in GitHub and
 not
 > ReviewB

Re: Reviews on Github

2016-09-21 Thread Horacio Duran
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

On Wednesday, 21 September 2016, Reed O'Brien 
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 > > 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.
>>> 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 >>> > 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 >> > 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
>>> > >> >
>>> 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
>

Re: Reviews on Github

2016-09-21 Thread Reed O'Brien
On Wed, Sep 21, 2016 at 4:43 PM, Horacio Duran 
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 
> 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.
>> 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 
>>> 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 >>> > 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  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
>> >  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  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

Re: Reviews on Github

2016-09-21 Thread Horacio Duran
I disagree, once the discussion is over and the merge is ready, an
amend/squash is in order to avoid useless tree nodes.

On Wed, Sep 21, 2016 at 8:41 PM, Reed O'Brien 
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.
> 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 
>> 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 
>>> 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  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
> >  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  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:

Re: Reviews on Github

2016-09-21 Thread Reed O'Brien
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.
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 
> 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 
>> 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  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
 >  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  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 unsubsc

Re: Reviews on Github

2016-09-21 Thread Andrew Wilkins
On Wed, Sep 21, 2016 at 5:53 AM Menno Smits 
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 
> 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  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
>>> >  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  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


Re: Reviews on Github

2016-09-20 Thread Casey Marshall
I had pretty high hopes for Github Reviews but Reviews are starting to show
some rough edges.

Comments on prior commits in the PR stick around. They never go away.
Before Juju Reviews, pushing a change would hide old lines, which was bad,
because it was easy to lose track of requests and comments. Now, we have an
opposite problem -- lots of noise because comments never get hidden, they
just lose their context. We've taken to deleting some of the really
low-signal threads, which isn't great.

If we could mark these comments with an actually useful marker, like DONE,
and have them collapse... No, but we get a slice of pizza or a party hat
sticker to put on it.

-Casey

On Tue, Sep 20, 2016 at 4:54 PM, Menno Smits 
wrote:

> (gah, hit send too early)
>
> ... If we decide to stay with RB, that will need to be fixed.
>
> On 21 September 2016 at 09:53, Menno Smits 
> 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.
>>
>> 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 
>> 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  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
 >  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  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/mailm
 an/listinfo/juju-dev

>>>
>>> --
>>> Juju-dev mailing list
>>> Juju-dev@lists.ubuntu.com
>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>>> an/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

Re: Reviews on Github

2016-09-20 Thread Nate Finch
Personally, I really enjoy having everything in the same place, more than I
expected. It's also one less barrier of entry for newcomers and outsiders.

On Tue, Sep 20, 2016, 5:54 PM Menno Smits  wrote:

> (gah, hit send too early)
>
> ... If we decide to stay with RB, that will need to be fixed.
>
> On 21 September 2016 at 09:53, Menno Smits 
> 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.
>>
>> 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 
>> 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  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
 >  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  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


Re: Reviews on Github

2016-09-20 Thread Menno Smits
(gah, hit send too early)

... If we decide to stay with RB, that will need to be fixed.

On 21 September 2016 at 09:53, Menno Smits 
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.
>
> 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 
> 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  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
>>> >  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  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/mailm
>>> an/listinfo/juju-dev
>>>
>>
>> --
>> Juju-dev mailing list
>> Juju-dev@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>> an/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


Re: Reviews on Github

2016-09-20 Thread Menno Smits
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.

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 
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  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  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
>> >  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  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


Re: Reviews on Github

2016-09-20 Thread Anastasia Macmood
The A-team (:D) decided to trial reviews on github for couple of weeks
too \o/

So far, we have been very happy with the new tool.

It'd be nice to have a quick retro at the end of this trial to pool
collective experiences of the teams.

Sincerely Yours,

Anastasia


On 21/09/16 03:54, Rick Harding 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
>  > 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  > 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
> >  > 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  > 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


Re: Reviews on Github

2016-09-20 Thread Rick Harding
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  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
> >  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  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


Re: Reviews on Github

2016-09-20 Thread Katherine Cox-Buday
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  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
>  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  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


Re: Reviews on Github

2016-09-20 Thread Nate Finch
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  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


Re: Reviews on Github

2016-09-20 Thread Katherine Cox-Buday
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  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


Re: Reviews on Github

2016-09-15 Thread Ian Booth

On 16/09/16 03:50, Nate Finch 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.
>

If we want to do a trial, and we acknowledge that there are functional gaps, and
we are prepared to work around those using convention, then we should document
what those conventions are so that everyone takes a consistent approach.


> 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.
> 

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Reviews on Github

2016-09-15 Thread Ian Booth


On 16/09/16 08:54, Anastasia Macmood wrote:
> 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 :)
>>

The point is that that's not always possible. The example given was where we
need to update import paths due to a dependency change. That has to be done all
in one go. There are other occasions as well where sometimes a mechanical change
needs to touch a lot of files in the one PR. We just need to be sure that any RB
replacement caters for those scenarios.


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Reviews on Github

2016-09-15 Thread Anastasia Macmood
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  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  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 
 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 

Re: Reviews on Github

2016-09-15 Thread Ian Booth
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.

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  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  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 
>>> 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

Re: Reviews on Github

2016-09-15 Thread Menno Smits
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  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  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 
>> 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
> M

Re: Reviews on Github

2016-09-15 Thread Nate Finch
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  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 
> 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


Re: Reviews on Github

2016-09-14 Thread Ian Booth


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  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


Re: Reviews on Github

2016-09-14 Thread Andrew Wilkins
On Thu, Sep 15, 2016 at 6:22 AM 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.
>

Agreed and +1. I propose we all try it for a couple of weeks, and see how
we feel about it then. RB isn't going to go anywhere soon - it's just a
matter of whether we keep our instance alive.

In case anyone's wondering about pipelines: it looks like you can review on
individual commits, so that's covered.

On Wed, Sep 14, 2016 at 6:13 PM Ian Booth  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


Re: Reviews on Github

2016-09-14 Thread Anastasia Macmood
+1 on moving away from RB \o/

Currently contributors need to allow RB to run against their github
fork, if they don't then we do not see their contributions on RB and PRs
go un-reviewed and seem ignored.

Communication between github and RB is not optimal: we had plenty of
instances where RB would close a review but github PR is still active;
as well as the other way around.

RB is also not configured on some of our "external" libraries under
github/juju. So we are not reviewing these proposals either...

Not to mention that we still need to fall back to github to confirm that
there are no conflicts on PRs as RB does not report that.


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. 
>
> On Wed, Sep 14, 2016 at 6:13 PM Ian Booth  > 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


Re: Reviews on Github

2016-09-14 Thread Rick Harding
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.

On Wed, Sep 14, 2016 at 6:13 PM Ian Booth  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


Re: Reviews on Github

2016-09-14 Thread Ian Booth
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


Re: Reviews on Github

2016-09-14 Thread Tim Penhey

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


Re: Reviews on Github

2016-09-14 Thread Dimiter Naydenov
As long as we can have draft reviews like on RB and not
email-spam-per-comment, totally +1

On 09/14/2016 01:25 PM, Horacio Duran wrote:
> Also +1 for that source not being review board
> 
> On Wed, Sep 14, 2016 at 5:23 PM, Reed O'Brien  > wrote:
> 
> Also +1 for a single source of truth.
> 
> On Wed, Sep 14, 2016 at 1:20 PM, Rick Harding
> mailto:rick.hard...@canonical.com>> wrote:
> 
> /me is always +1 on reducing the number of things we have to
> maintain and keeping things simpler. 
> 
> On Wed, Sep 14, 2016 at 4:04 PM Nate Finch
> mailto:nate.fi...@canonical.com>> 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
> 
> 
> 
> 
> 
> -- 
> 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
> 
> 
> 
> 
> 


-- 
Dimiter Naydenov 
Juju Core Sapphire team 



signature.asc
Description: OpenPGP digital signature
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Reviews on Github

2016-09-14 Thread Casey Marshall
I'm halfway through my first Github review (different project though) on
the new system, and so far I'm loving it. Also consider the issues we've
had with rbt being unable to handle diffs with files
added/removed/relocated. +1 from me!

-Casey

On Wed, Sep 14, 2016 at 3:23 PM, Reed O'Brien 
wrote:

> Also +1 for a single source of truth.
>
> On Wed, Sep 14, 2016 at 1:20 PM, Rick Harding 
> wrote:
>
>> /me is always +1 on reducing the number of things we have to maintain and
>> keeping things simpler.
>>
>> On Wed, Sep 14, 2016 at 4:04 PM 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.co
>>> m/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/mailm
>>> an/listinfo/juju-dev
>>>
>>
>> --
>> Juju-dev mailing list
>> Juju-dev@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>> an/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
>
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Reviews on Github

2016-09-14 Thread Horacio Duran
Also +1 for that source not being review board

On Wed, Sep 14, 2016 at 5:23 PM, Reed O'Brien 
wrote:

> Also +1 for a single source of truth.
>
> On Wed, Sep 14, 2016 at 1:20 PM, Rick Harding 
> wrote:
>
>> /me is always +1 on reducing the number of things we have to maintain and
>> keeping things simpler.
>>
>> On Wed, Sep 14, 2016 at 4:04 PM 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.co
>>> m/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/mailm
>>> an/listinfo/juju-dev
>>>
>>
>> --
>> Juju-dev mailing list
>> Juju-dev@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>> an/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
>
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Reviews on Github

2016-09-14 Thread Reed O'Brien
Also +1 for a single source of truth.

On Wed, Sep 14, 2016 at 1:20 PM, Rick Harding 
wrote:

> /me is always +1 on reducing the number of things we have to maintain and
> keeping things simpler.
>
> On Wed, Sep 14, 2016 at 4:04 PM 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
>
>


-- 
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


Re: Reviews on Github

2016-09-14 Thread Rick Harding
/me is always +1 on reducing the number of things we have to maintain and
keeping things simpler.

On Wed, Sep 14, 2016 at 4:04 PM 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