Re: Reviewboard - dependant branches
On 18/12/14 03:01, John Weldon wrote: Can someone remind me of the process for proposing branches that depend on another branch that is still in review? I know it's been discussed here and on irc before, but I don't recall the specifics. I think that the steps are: 1. Just propose the branch in github, which will trigger the automatic reviewboard integration 2. run some rbt command line invocation to update reviewboard with the upstream branch name I just don't recall the exact invocation for #2 and I don't want to experiment and muck something up :) rbt post -u --parent Cheers, -- John Weldon -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard update
FWIW, I'd rather we used HTTPS and just fixed whatever issues we had with Ship It, etc. But I'm certainly happiest just to see it working. John =:-> On Mon, Nov 17, 2014 at 6:55 PM, Eric Snow wrote: > On Mon, Nov 17, 2014 at 5:10 AM, Dimiter Naydenov > wrote: > > I can confirm RB diffs for backports to 1.21 get generated correctly > > now, and the PR description is updated to include a link to the RB diff. > > Thanks for reporting on this. > > > > > There's one issue however -- the link in the PR is > > "https://reviews.vapour.ws/r/", rather than "http://..";, which > > cause some problems: > > I've switched it to HTTP. > > -eric > > -- > 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: reviewboard update
On Mon, Nov 17, 2014 at 5:10 AM, Dimiter Naydenov wrote: > I can confirm RB diffs for backports to 1.21 get generated correctly > now, and the PR description is updated to include a link to the RB diff. Thanks for reporting on this. > > There's one issue however -- the link in the PR is > "https://reviews.vapour.ws/r/", rather than "http://..";, which > cause some problems: I've switched it to HTTP. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard update
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 15.11.2014 07:07, Eric Snow wrote: > FYI, I was able to solve 3 reviewboard-github integration issues > today: > > 1. pull requests for branches other than master now work (e.g. 1.21 > backports) 2. no more hitting rate limits (5000 requests/hour limit > instead of 60) 3. pull request bodies now get updated with a link > to the new review request > > If you have any trouble with any of these please let me know. > Thanks! > > -eric > Great job Eric! I can confirm RB diffs for backports to 1.21 get generated correctly now, and the PR description is updated to include a link to the RB diff. There's one issue however -- the link in the PR is "https://reviews.vapour.ws/r/", rather than "http://..";, which cause some problems: 1. The browser displays "The site's security certificate is not trusted", so I need to either add an exception or change the URL to use "http" instead. 2. If you do add an exception try to review and click "Ship It!", it's not taking effect (Michael had that issue with one of my reviews). So you *need* to use the http URL for the "Ship It!" to work. It would be nice to fix this :) Thanks, - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUaeWaAAoJENzxV2TbLzHw2kcH/iddKLRScZFgBue9RqcEr5DU XrsSin07R1SrrD7xK/PH9z8ONGCVcXeZyAG+hkZiIu7cmL69pFs/ooOuLPZRHuSs ZmEGi27WMX9Aem4RkJpgjet1xMW1eOAeSYgSHeaqPIzSPWwArl6HedUw0V40/etW 3MJJPQw0FpCl6iEqqJtq85s+Ngs8n58Dk0dAQt7yVKQ1xovoognWwdGrTcio2NIZ y5dDM1c+1OJfkRkZDMUAQpJBq45uit29kDc9XAgHUlWZf/1tXD8SLslLb9Wiz7Us b+fQaP+2L+JjnVdN1gObkhMUF67rJceX1Y5djiEubAp6/RTKaMsHuEZT5O2zERM= =kGql -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard update
Awesome, nice work! On Nov 15, 2014 12:07 AM, "Eric Snow" wrote: > FYI, I was able to solve 3 reviewboard-github integration issues today: > > 1. pull requests for branches other than master now work (e.g. 1.21 > backports) > 2. no more hitting rate limits (5000 requests/hour limit instead of 60) > 3. pull request bodies now get updated with a link to the new review > request > > If you have any trouble with any of these please let me know. Thanks! > > -eric > > -- > 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: reviewboard - most recent diff by default?
On Mon, Oct 27, 2014 at 9:05 PM, Jesse Meek wrote: > Is possible and preferable to show the most recent diff by default? If you mean instead of showing the "reviews" page by default, ReviewBoard doesn't support that out of the box. Certainly we could customize RB to do so, and for all I know it would be easy. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On Tue, Oct 21, 2014 at 10:12 AM, Eric Snow wrote: > For now I've hard-coded adding juju-team. If anyone still has trouble > with this please let me know. The issue with not finding files in the repo should be fixed now. This means that auto-generated review requests should have diffs and be published automatically. If you have any trouble with it, let me know. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On Tue, Oct 21, 2014 at 3:40 AM, Michael Foord wrote: > > On 20/10/14 22:38, Eric Snow wrote: >> >> This should be resolved now. I've verified it works for me. If it >> still impacts anyone, just let me know. > > > I still have the issue I'm afraid. No reviewer set, no diff. > > http://reviews.vapour.ws/r/211/ For now I've hard-coded adding juju-team. If anyone still has trouble with this please let me know. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On Tue, Oct 21, 2014 at 11:40 AM, Michael Foord wrote: > > On 20/10/14 22:38, Eric Snow wrote: >> >> This should be resolved now. I've verified it works for me. If it >> still impacts anyone, just let me know. > > > I still have the issue I'm afraid. No reviewer set, no diff. > > http://reviews.vapour.ws/r/211/ > > Michael :( The frustrating part is that I haven't been impacted by this. I'll hack a solution in the immediate term and go from there. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On 20/10/14 22:38, Eric Snow wrote: This should be resolved now. I've verified it works for me. If it still impacts anyone, just let me know. I still have the issue I'm afraid. No reviewer set, no diff. http://reviews.vapour.ws/r/211/ Michael -eric On Mon, Oct 20, 2014 at 7:34 PM, Eric Snow wrote: Yeah, this is the same issue that Ian brought up. I'm looking into it. Sorry for the pain. -eric On Mon, Oct 20, 2014 at 5:31 PM, Dimiter Naydenov wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hey Eric, Today I tried proposing a PR and the RB issue (#202) was created, but it didn't have "Reviewers" field set (as described below), it wasn't published (due to the former), but MOST importantly didn't have a diff uploaded. After fiddling around with rbt I managed to do: $ rbt diff > ~/patch (while on the proposed feature branch) And then went to the RB issue page and manually uploaded the generated diff and published it. So most definitely the hook generating RB issues have to upload the diff as well :) It's coming together, keep up the good work! Cheers, Dimiter On 20.10.2014 16:53, Eric Snow wrote: On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth wrote: Hey Eric This is awesome, thank you. I did run into a gotcha - I created a PR and then looked at the Incoming review queue and there was nothing new there. I then clicked on All in the Outgoing review queue and saw that the review was unpublished. I then went to publish it and it complained at least one reviewer was needed. So I had to fill in "juju-team" and all was good. 1. Can we make it so that the review is published automatically? 2. Can we pre-fill "juju-team" as the reviewer? Good catch. The two are actually related. The review is published, but that fails because no reviewer got set. I'll get that fixed. -eric - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJURSrnAAoJENzxV2TbLzHw0BQH/16P4qPDI28kkGs398qRKY5s eUtcHBpYs+JuLV2ZA0LjCpTds89RBDW6cKsxcfXxaAmawIb0KHh920VzKb1Wl2OT z/iMOF2q91LnV58dqPf7mZjHaT1LPRdSRxg6aAZW/mjexwVRtRDT4Asd5w6JpKrH 9Tkqfy86OilJ70X8qNbegvjJrBAttwoLLI4jwJq4dNWUbWCBbuumryh0k6+GlmNH NiKbpi45pPy/RIFVA7ewbLIOpUXleHm5NIGlA/liZOMHpz0w5QHK3FYGLuGMNzQC fq4qW6rfb1ITdr7XWsA3gooV6FUndw3mbNsod3QgSv82RDA6GGECHeYimGG94/g= =POJ4 -END PGP SIGNATURE- -- 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: reviewboard-github integration
Hi Eric I just created a pull request for a 1.20 branch and got the same symptoms as seen previously. ie an incomplete review board review without a diff and with a reviewer. On 21/10/14 07:38, Eric Snow wrote: > This should be resolved now. I've verified it works for me. If it > still impacts anyone, just let me know. > > -eric > > On Mon, Oct 20, 2014 at 7:34 PM, Eric Snow wrote: >> Yeah, this is the same issue that Ian brought up. I'm looking into >> it. Sorry for the pain. >> >> -eric >> >> On Mon, Oct 20, 2014 at 5:31 PM, Dimiter Naydenov >> wrote: > Hey Eric, > > Today I tried proposing a PR and the RB issue (#202) was created, but > it didn't have "Reviewers" field set (as described below), it wasn't > published (due to the former), but MOST importantly didn't have a diff > uploaded. After fiddling around with rbt I managed to do: > $ rbt diff > ~/patch > (while on the proposed feature branch) > > And then went to the RB issue page and manually uploaded the generated > diff and published it. > > So most definitely the hook generating RB issues have to upload the > diff as well :) > > It's coming together, keep up the good work! > > Cheers, > Dimiter > > On 20.10.2014 16:53, Eric Snow wrote: > On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth > wrote: >> Hey Eric >> >> This is awesome, thank you. >> >> I did run into a gotcha - I created a PR and then looked at the >> Incoming review queue and there was nothing new there. I then >> clicked on All in the Outgoing review queue and saw that the >> review was unpublished. I then went to publish it and it >> complained at least one reviewer was needed. So I had to fill in >> "juju-team" and all was good. >> >> 1. Can we make it so that the review is published automatically? >> 2. Can we pre-fill "juju-team" as the reviewer? > > Good catch. The two are actually related. The review is > published, but that fails because no reviewer got set. I'll get > that fixed. > > -eric > > > >>> >>> -- >>> 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: reviewboard-github integration
This should be resolved now. I've verified it works for me. If it still impacts anyone, just let me know. -eric On Mon, Oct 20, 2014 at 7:34 PM, Eric Snow wrote: > Yeah, this is the same issue that Ian brought up. I'm looking into > it. Sorry for the pain. > > -eric > > On Mon, Oct 20, 2014 at 5:31 PM, Dimiter Naydenov > wrote: >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA1 >> >> Hey Eric, >> >> Today I tried proposing a PR and the RB issue (#202) was created, but >> it didn't have "Reviewers" field set (as described below), it wasn't >> published (due to the former), but MOST importantly didn't have a diff >> uploaded. After fiddling around with rbt I managed to do: >> $ rbt diff > ~/patch >> (while on the proposed feature branch) >> >> And then went to the RB issue page and manually uploaded the generated >> diff and published it. >> >> So most definitely the hook generating RB issues have to upload the >> diff as well :) >> >> It's coming together, keep up the good work! >> >> Cheers, >> Dimiter >> >> On 20.10.2014 16:53, Eric Snow wrote: >>> On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth >>> wrote: Hey Eric This is awesome, thank you. I did run into a gotcha - I created a PR and then looked at the Incoming review queue and there was nothing new there. I then clicked on All in the Outgoing review queue and saw that the review was unpublished. I then went to publish it and it complained at least one reviewer was needed. So I had to fill in "juju-team" and all was good. 1. Can we make it so that the review is published automatically? 2. Can we pre-fill "juju-team" as the reviewer? >>> >>> Good catch. The two are actually related. The review is >>> published, but that fails because no reviewer got set. I'll get >>> that fixed. >>> >>> -eric >>> >> >> >> - -- >> Dimiter Naydenov >> juju-core team >> -BEGIN PGP SIGNATURE- >> Version: GnuPG v1 >> >> iQEcBAEBAgAGBQJURSrnAAoJENzxV2TbLzHw0BQH/16P4qPDI28kkGs398qRKY5s >> eUtcHBpYs+JuLV2ZA0LjCpTds89RBDW6cKsxcfXxaAmawIb0KHh920VzKb1Wl2OT >> z/iMOF2q91LnV58dqPf7mZjHaT1LPRdSRxg6aAZW/mjexwVRtRDT4Asd5w6JpKrH >> 9Tkqfy86OilJ70X8qNbegvjJrBAttwoLLI4jwJq4dNWUbWCBbuumryh0k6+GlmNH >> NiKbpi45pPy/RIFVA7ewbLIOpUXleHm5NIGlA/liZOMHpz0w5QHK3FYGLuGMNzQC >> fq4qW6rfb1ITdr7XWsA3gooV6FUndw3mbNsod3QgSv82RDA6GGECHeYimGG94/g= >> =POJ4 >> -END PGP SIGNATURE- >> >> -- >> 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: reviewboard-github integration
Yeah, this is the same issue that Ian brought up. I'm looking into it. Sorry for the pain. -eric On Mon, Oct 20, 2014 at 5:31 PM, Dimiter Naydenov wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > Hey Eric, > > Today I tried proposing a PR and the RB issue (#202) was created, but > it didn't have "Reviewers" field set (as described below), it wasn't > published (due to the former), but MOST importantly didn't have a diff > uploaded. After fiddling around with rbt I managed to do: > $ rbt diff > ~/patch > (while on the proposed feature branch) > > And then went to the RB issue page and manually uploaded the generated > diff and published it. > > So most definitely the hook generating RB issues have to upload the > diff as well :) > > It's coming together, keep up the good work! > > Cheers, > Dimiter > > On 20.10.2014 16:53, Eric Snow wrote: >> On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth >> wrote: >>> Hey Eric >>> >>> This is awesome, thank you. >>> >>> I did run into a gotcha - I created a PR and then looked at the >>> Incoming review queue and there was nothing new there. I then >>> clicked on All in the Outgoing review queue and saw that the >>> review was unpublished. I then went to publish it and it >>> complained at least one reviewer was needed. So I had to fill in >>> "juju-team" and all was good. >>> >>> 1. Can we make it so that the review is published automatically? >>> 2. Can we pre-fill "juju-team" as the reviewer? >> >> Good catch. The two are actually related. The review is >> published, but that fails because no reviewer got set. I'll get >> that fixed. >> >> -eric >> > > > - -- > Dimiter Naydenov > juju-core team > -BEGIN PGP SIGNATURE- > Version: GnuPG v1 > > iQEcBAEBAgAGBQJURSrnAAoJENzxV2TbLzHw0BQH/16P4qPDI28kkGs398qRKY5s > eUtcHBpYs+JuLV2ZA0LjCpTds89RBDW6cKsxcfXxaAmawIb0KHh920VzKb1Wl2OT > z/iMOF2q91LnV58dqPf7mZjHaT1LPRdSRxg6aAZW/mjexwVRtRDT4Asd5w6JpKrH > 9Tkqfy86OilJ70X8qNbegvjJrBAttwoLLI4jwJq4dNWUbWCBbuumryh0k6+GlmNH > NiKbpi45pPy/RIFVA7ewbLIOpUXleHm5NIGlA/liZOMHpz0w5QHK3FYGLuGMNzQC > fq4qW6rfb1ITdr7XWsA3gooV6FUndw3mbNsod3QgSv82RDA6GGECHeYimGG94/g= > =POJ4 > -END PGP SIGNATURE- > > -- > 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: reviewboard-github integration
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hey Eric, Today I tried proposing a PR and the RB issue (#202) was created, but it didn't have "Reviewers" field set (as described below), it wasn't published (due to the former), but MOST importantly didn't have a diff uploaded. After fiddling around with rbt I managed to do: $ rbt diff > ~/patch (while on the proposed feature branch) And then went to the RB issue page and manually uploaded the generated diff and published it. So most definitely the hook generating RB issues have to upload the diff as well :) It's coming together, keep up the good work! Cheers, Dimiter On 20.10.2014 16:53, Eric Snow wrote: > On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth > wrote: >> Hey Eric >> >> This is awesome, thank you. >> >> I did run into a gotcha - I created a PR and then looked at the >> Incoming review queue and there was nothing new there. I then >> clicked on All in the Outgoing review queue and saw that the >> review was unpublished. I then went to publish it and it >> complained at least one reviewer was needed. So I had to fill in >> "juju-team" and all was good. >> >> 1. Can we make it so that the review is published automatically? >> 2. Can we pre-fill "juju-team" as the reviewer? > > Good catch. The two are actually related. The review is > published, but that fails because no reviewer got set. I'll get > that fixed. > > -eric > - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJURSrnAAoJENzxV2TbLzHw0BQH/16P4qPDI28kkGs398qRKY5s eUtcHBpYs+JuLV2ZA0LjCpTds89RBDW6cKsxcfXxaAmawIb0KHh920VzKb1Wl2OT z/iMOF2q91LnV58dqPf7mZjHaT1LPRdSRxg6aAZW/mjexwVRtRDT4Asd5w6JpKrH 9Tkqfy86OilJ70X8qNbegvjJrBAttwoLLI4jwJq4dNWUbWCBbuumryh0k6+GlmNH NiKbpi45pPy/RIFVA7ewbLIOpUXleHm5NIGlA/liZOMHpz0w5QHK3FYGLuGMNzQC fq4qW6rfb1ITdr7XWsA3gooV6FUndw3mbNsod3QgSv82RDA6GGECHeYimGG94/g= =POJ4 -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On Mon, Oct 20, 2014 at 6:06 AM, Ian Booth wrote: > Hey Eric > > This is awesome, thank you. > > I did run into a gotcha - I created a PR and then looked at the Incoming > review > queue and there was nothing new there. I then clicked on All in the Outgoing > review queue and saw that the review was unpublished. I then went to publish > it > and it complained at least one reviewer was needed. So I had to fill in > "juju-team" and all was good. > > 1. Can we make it so that the review is published automatically? > 2. Can we pre-fill "juju-team" as the reviewer? Good catch. The two are actually related. The review is published, but that fails because no reviewer got set. I'll get that fixed. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
Hey Eric This is awesome, thank you. I did run into a gotcha - I created a PR and then looked at the Incoming review queue and there was nothing new there. I then clicked on All in the Outgoing review queue and saw that the review was unpublished. I then went to publish it and it complained at least one reviewer was needed. So I had to fill in "juju-team" and all was good. 1. Can we make it so that the review is published automatically? 2. Can we pre-fill "juju-team" as the reviewer? On 18/10/14 15:38, Eric Snow wrote: > With the switch to Reviewboard we introduced extra steps to our > workflow (mostly involving rbt). This in turn made things more > difficult for new/existing contributors. I've been able to take some > time in the last couple weeks to improve the situation by adding some > integration between github and reviewboard. > > As of tonight that integration has reached an initial milestone. The > barriers to contribution introduced by Reviewboard are essentially > gone. Furthermore, the automation means the review requests should > stay in sync with the pull requests. So I'm happy to report that, > unless you are chaining branches (which github PRs don't support > anyway), you shouldn't need to use rbt anymore. > > Currently: > > * a new PR automatically triggers the creation of a new review request > * the review request has a link back to the pull request > * updates to the PR (i.e. pushes to your branch) automatically trigger > an update to the review request > * closing (discard/merge) a PR automatically triggers closing the > corresponding review request > * re-opening a PR automatically triggers re-opening the corresponding > review request > * a reviewboard user gets created if there wasn't one already > > Nearly working: > > * after the review request is created, a link to it is added to a pull > request comment > > Future work: > > * support patch queues/chained branches/etc. (via trigger in PR summary) > * add reviewboard support to the merge bot (check for ship-it before > doing anything) > > Will not happen: > > * automatically merge PR when given ship-it > * PR comments (including review comments) will not be pushed to the > corresponding review request > * likewise reviewboard comments won't be pushed up to the corresponding PR > > I can't promise that the "future" work will happen in the short term, > but I'll post any updates as they come. Enjoy! > > -eric > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
And of course if you see any problems please let me know. :) -eric On Sat, Oct 18, 2014 at 7:38 AM, Eric Snow wrote: > With the switch to Reviewboard we introduced extra steps to our > workflow (mostly involving rbt). This in turn made things more > difficult for new/existing contributors. I've been able to take some > time in the last couple weeks to improve the situation by adding some > integration between github and reviewboard. > > As of tonight that integration has reached an initial milestone. The > barriers to contribution introduced by Reviewboard are essentially > gone. Furthermore, the automation means the review requests should > stay in sync with the pull requests. So I'm happy to report that, > unless you are chaining branches (which github PRs don't support > anyway), you shouldn't need to use rbt anymore. > > Currently: > > * a new PR automatically triggers the creation of a new review request > * the review request has a link back to the pull request > * updates to the PR (i.e. pushes to your branch) automatically trigger > an update to the review request > * closing (discard/merge) a PR automatically triggers closing the > corresponding review request > * re-opening a PR automatically triggers re-opening the corresponding > review request > * a reviewboard user gets created if there wasn't one already > > Nearly working: > > * after the review request is created, a link to it is added to a pull > request comment > > Future work: > > * support patch queues/chained branches/etc. (via trigger in PR summary) > * add reviewboard support to the merge bot (check for ship-it before > doing anything) > > Will not happen: > > * automatically merge PR when given ship-it > * PR comments (including review comments) will not be pushed to the > corresponding review request > * likewise reviewboard comments won't be pushed up to the corresponding PR > > I can't promise that the "future" work will happen in the short term, > but I'll post any updates as they come. Enjoy! > > -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
On Sat, Oct 18, 2014 at 1:38 PM, Eric Snow wrote: > With the switch to Reviewboard we introduced extra steps to our > workflow (mostly involving rbt). This in turn made things more > difficult for new/existing contributors. I've been able to take some > time in the last couple weeks to improve the situation by adding some > integration between github and reviewboard. > > As of tonight that integration has reached an initial milestone. The > barriers to contribution introduced by Reviewboard are essentially > gone. Furthermore, the automation means the review requests should > stay in sync with the pull requests. So I'm happy to report that, > unless you are chaining branches (which github PRs don't support > anyway), you shouldn't need to use rbt anymore. > > Currently: > > * a new PR automatically triggers the creation of a new review request > * the review request has a link back to the pull request > * updates to the PR (i.e. pushes to your branch) automatically trigger > an update to the review request > * closing (discard/merge) a PR automatically triggers closing the > corresponding review request > * re-opening a PR automatically triggers re-opening the corresponding > review request > * a reviewboard user gets created if there wasn't one already > > Nearly working: > > * after the review request is created, a link to it is added to a pull > request comment > > Future work: > > * support patch queues/chained branches/etc. (via trigger in PR summary) > * add reviewboard support to the merge bot (check for ship-it before > doing anything) > > Will not happen: > > * automatically merge PR when given ship-it > * PR comments (including review comments) will not be pushed to the > corresponding review request > * likewise reviewboard comments won't be pushed up to the corresponding PR > > I can't promise that the "future" work will happen in the short term, > but I'll post any updates as they come. Enjoy! > > -eric > Very nice. Thanks, Eric. Cheers, Andrew -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard-github integration
Also, I did this as a reviewboard extension. All the code is online (BSD license): https://bitbucket.org/ericsnowcurrently/rb_webhooks_extension. I haven't charmed it up, but it should be pretty easy to adapt the charm I wrote for rb_oauth. -eric On Sat, Oct 18, 2014 at 7:38 AM, Eric Snow wrote: > With the switch to Reviewboard we introduced extra steps to our > workflow (mostly involving rbt). This in turn made things more > difficult for new/existing contributors. I've been able to take some > time in the last couple weeks to improve the situation by adding some > integration between github and reviewboard. > > As of tonight that integration has reached an initial milestone. The > barriers to contribution introduced by Reviewboard are essentially > gone. Furthermore, the automation means the review requests should > stay in sync with the pull requests. So I'm happy to report that, > unless you are chaining branches (which github PRs don't support > anyway), you shouldn't need to use rbt anymore. > > Currently: > > * a new PR automatically triggers the creation of a new review request > * the review request has a link back to the pull request > * updates to the PR (i.e. pushes to your branch) automatically trigger > an update to the review request > * closing (discard/merge) a PR automatically triggers closing the > corresponding review request > * re-opening a PR automatically triggers re-opening the corresponding > review request > * a reviewboard user gets created if there wasn't one already > > Nearly working: > > * after the review request is created, a link to it is added to a pull > request comment > > Future work: > > * support patch queues/chained branches/etc. (via trigger in PR summary) > * add reviewboard support to the merge bot (check for ship-it before > doing anything) > > Will not happen: > > * automatically merge PR when given ship-it > * PR comments (including review comments) will not be pushed to the > corresponding review request > * likewise reviewboard comments won't be pushed up to the corresponding PR > > I can't promise that the "future" work will happen in the short term, > but I'll post any updates as they come. Enjoy! > > -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On 16 September 2014 13:45, David Cheney wrote: > On Tue, Sep 16, 2014 at 7:27 PM, roger peppe > wrote: >> On 16 September 2014 09:22, Jonathan Aquilina >> wrote: >>> If i am not mistaken if you have multiple commits in a branch git has >>> something built in called git squash. This obviously eliminates the 5 step >>> process into one merge and one push. >> >> I don't see that command. Are you thinking of the "squash" >> functionality of rebase -i? >> >> FWIW, I never run those five steps in sequence together. >> Usually I just get to a situation where I know that I have all tests >> passing and I'm up to date with master (for example I've done a merge >> some time ago, probably before fixing a bunch of tests). >> >> Then it's just: >> >> $ git reset upstream/master >> $ git commit -am 'my commit message' > > Nice trick, so that resets the underlying branch to master, leaving > everything you have comitted or merged uncomitted ? Yes. Specifically, it sets the branch *head* to the same as master - the current branch name does not change. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On Tue, Sep 16, 2014 at 7:27 PM, roger peppe wrote: > On 16 September 2014 09:22, Jonathan Aquilina wrote: >> If i am not mistaken if you have multiple commits in a branch git has >> something built in called git squash. This obviously eliminates the 5 step >> process into one merge and one push. > > I don't see that command. Are you thinking of the "squash" > functionality of rebase -i? > > FWIW, I never run those five steps in sequence together. > Usually I just get to a situation where I know that I have all tests > passing and I'm up to date with master (for example I've done a merge > some time ago, probably before fixing a bunch of tests). > > Then it's just: > > $ git reset upstream/master > $ git commit -am 'my commit message' Nice trick, so that resets the underlying branch to master, leaving everything you have comitted or merged uncomitted ? > > which is usually quicker than running rebase -i, and much > quicker if the rebase replay leads to conflicts. > > cheers, > rog. > > -- > 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: ReviewBoard is now the official review tool for juju
That is it indeed :) --- Regards, Jonathan Aquilina Founder Eagle Eye T On 2014-09-16 11:58, Dimiter Naydenov wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 16.09.2014 12:32, Jonathan Aquilina wrote: > >> I dont think you have to rebase though. I think you can squash multiple commits together. > > You're probably thinking about git commit --amend -m "msg", which > folds the current changeset into the one before it, effectively > squashing the uncommitted changes onto the same revision ID, and > changing the commit message. > >> --- Regards, Jonathan Aquilina Founder Eagle Eye T On 2014-09-16 11:27, roger peppe wrote: >> >>> On 16 September 2014 09:22, Jonathan Aquilina > wrote: >>> If i am not mistaken if you have multiple commits in a branch git has something built in called git squash. This obviously eliminates the 5 step process into one merge and one push. >>> I don't see that command. Are you thinking of the "squash" functionality of rebase -i? FWIW, I never run those five steps in sequence together. Usually I just get to a situation where I know that I have all tests passing and I'm up to date with master (for example I've done a merge some time ago, probably before fixing a bunch of tests). Then it's just: $ git reset upstream/master $ git commit -am 'my commit message' which is usually quicker than running rebase -i, and much quicker if the rebase replay leads to conflicts. cheers, rog. > > - -- > Dimiter Naydenov > juju-core team > -BEGIN PGP SIGNATURE- > Version: GnuPG v1 > > iQEcBAEBAgAGBQJUGAm+AAoJENzxV2TbLzHwH74IAIUUxdHCS2KTvTNPRRAZt9F1 > r3/gLD/guBbicVuE/p7Ghj/DirmjuLDF8ZndmWO8EeIVye3ikw204lFklfpx4uSh > Qik5Mp+JPrFZ1u78n8EKhvh7gX+FytKooFSdUHjPfkxPTVZ2Rxg/LyXJSsp3WlvJ > G+wgx01gKztqaE37EhFQkYoE1+ULUm8phOTS6UhzVMiaRr+x7u4oL1M0i48+FHvC > R6KHBq0zoxijLxRgN7jfKpavPEIrCPEjTB/zOBN9NONKhVABlIU3HWb5JtBA+vVV > TbpmpKOSKdS7hNMnr2D/phKK62TxEpefHszmKWzuR/JWmq5cC7lEFwjUbUV8nFg= > =vcVP > -END PGP SIGNATURE- Links: -- [1] mailto:jaquil...@eagleeyet.net -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 16.09.2014 12:32, Jonathan Aquilina wrote: > I dont think you have to rebase though. I think you can squash > multiple commits together. > You're probably thinking about git commit --amend -m "msg", which folds the current changeset into the one before it, effectively squashing the uncommitted changes onto the same revision ID, and changing the commit message. > > > --- Regards, Jonathan Aquilina Founder Eagle Eye T > > On 2014-09-16 11:27, roger peppe wrote: > >> On 16 September 2014 09:22, Jonathan Aquilina >> mailto:jaquil...@eagleeyet.net>> >> wrote: >>> If i am not mistaken if you have multiple commits in a branch >>> git has something built in called git squash. This obviously >>> eliminates the 5 step process into one merge and one push. >> I don't see that command. Are you thinking of the "squash" >> functionality of rebase -i? >> >> FWIW, I never run those five steps in sequence together. Usually >> I just get to a situation where I know that I have all tests >> passing and I'm up to date with master (for example I've done a >> merge some time ago, probably before fixing a bunch of tests). >> >> Then it's just: >> >> $ git reset upstream/master $ git commit -am 'my commit message' >> >> which is usually quicker than running rebase -i, and much quicker >> if the rebase replay leads to conflicts. >> >> cheers, rog. > > - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUGAm+AAoJENzxV2TbLzHwH74IAIUUxdHCS2KTvTNPRRAZt9F1 r3/gLD/guBbicVuE/p7Ghj/DirmjuLDF8ZndmWO8EeIVye3ikw204lFklfpx4uSh Qik5Mp+JPrFZ1u78n8EKhvh7gX+FytKooFSdUHjPfkxPTVZ2Rxg/LyXJSsp3WlvJ G+wgx01gKztqaE37EhFQkYoE1+ULUm8phOTS6UhzVMiaRr+x7u4oL1M0i48+FHvC R6KHBq0zoxijLxRgN7jfKpavPEIrCPEjTB/zOBN9NONKhVABlIU3HWb5JtBA+vVV TbpmpKOSKdS7hNMnr2D/phKK62TxEpefHszmKWzuR/JWmq5cC7lEFwjUbUV8nFg= =vcVP -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
I dont think you have to rebase though. I think you can squash multiple commits together. --- Regards, Jonathan Aquilina Founder Eagle Eye T On 2014-09-16 11:27, roger peppe wrote: > On 16 September 2014 09:22, Jonathan Aquilina wrote: > >> If i am not mistaken if you have multiple commits in a branch git has something built in called git squash. This obviously eliminates the 5 step process into one merge and one push. > > I don't see that command. Are you thinking of the "squash" > functionality of rebase -i? > > FWIW, I never run those five steps in sequence together. > Usually I just get to a situation where I know that I have all tests > passing and I'm up to date with master (for example I've done a merge > some time ago, probably before fixing a bunch of tests). > > Then it's just: > > $ git reset upstream/master > $ git commit -am 'my commit message' > > which is usually quicker than running rebase -i, and much > quicker if the rebase replay leads to conflicts. > > cheers, > rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On 16 September 2014 09:22, Jonathan Aquilina wrote: > If i am not mistaken if you have multiple commits in a branch git has > something built in called git squash. This obviously eliminates the 5 step > process into one merge and one push. I don't see that command. Are you thinking of the "squash" functionality of rebase -i? FWIW, I never run those five steps in sequence together. Usually I just get to a situation where I know that I have all tests passing and I'm up to date with master (for example I've done a merge some time ago, probably before fixing a bunch of tests). Then it's just: $ git reset upstream/master $ git commit -am 'my commit message' which is usually quicker than running rebase -i, and much quicker if the rebase replay leads to conflicts. cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
If i am not mistaken if you have multiple commits in a branch git has something built in called git squash. This obviously eliminates the 5 step process into one merge and one push. --- Regards, Jonathan Aquilina Founder Eagle Eye T On 2014-09-16 09:44, roger peppe wrote: > On 15 September 2014 21:39, Ian Booth wrote: > >> On 16/09/14 00:50, Eric Snow wrote: >> >>> On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow wrote: >>> Yeah, those steps are a lot, though keep in mind that effectively it's only 2 steps more than before if you use the -p flag to rbt post and were already keeping your local master up to date. >>> Just to be clear, here are the steps again, slightly reformatted: (0). Rebase relative to upstream master. - if origin is different than upstream, sync and push it >> Before I create a new branch, I ensure my local and origin (forked copy) master branches are up to date. However, once the branch is created, thereafter I do not rebase because it has caused nothing but trouble, with lots of manual effort required to fix things up wrt conflicts etc. > > Here's a trick I use (I imagine it's a well > known technique) that makes it easy to do a lightweight > rebase without the conflicts that often come when rebase > replays your commits: > > $ git fetch upstream > $ git merge upstream/master > $ git reset upstream/master > $ git add any new files added in your branch > $ git commit -am 'describe your branch' > > As far as I can make out, as long as you want to > propose your branch with only a single commit > added to the log, this makes it easy to use a > merge-based flow and amounts to > the same thing in the end. > > I use it a lot, and it's saved no end of "this is the *third* time > I've resolved these conflicts" pain. > > cheers, > rog. Links: -- [1] mailto:eric.s...@canonical.com -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 16.09.2014 10:44, roger peppe wrote: > As far as I can make out, as long as you want to propose your > branch with only a single commit added to the log, this makes it > easy to use a merge-based flow and amounts to the same thing in the > end. > > I use it a lot, and it's saved no end of "this is the *third* time > I've resolved these conflicts" pain. > Yes, using the merge workflow you quite often have to deal with resolving the same conflicts each time. And at the end you have a bunch of merge changesets, which is not very helpful for following the commit log later. Just two reasons I'm using the rebase workflow, each time I fetch master, I do an interactive rebase and usually leave only one commit (folding the others in it). One more thought re "rebase considered evil/impolite" - I only do that in my own branches, which is not a problem (even after the initial push, I still rebase, despite the branch going "public"). I guess this won't work well if you have to collaborate with other people on the same branch, but this is by far less common scenario. Cheers, - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUF+3+AAoJENzxV2TbLzHwLwEH/0VhdicdszrULpWma00mIYB0 G5xZnDEEXsB2Ud1EBhvLm84OIQeIfpuL/osbPEeb9xR1w8856Gll36nbNr8wuQle S+Wq/Gj3VT5jYo7TthUem6qG415/FLa9erPYmCXbyNynXHdBUA5Y8eTbRCZSUC0a kEdUot/vs5aPlZnFgvrxPtKZ2J7D37BslhZZujKQrDeMEoQHtZ9fgYKHDJovE3Yn 9TGcqX+d8mYxQrOD1GkBxcjEbt919ti3s3cqZDTzi/7zeGgXfCjMGoG4gEuPX5Jv O1IHjIqYxJRx0K5qkGimKtIMYe15WOEA4dCkd9p6pdLMh690lqwYwdp1MqVYlEo= =RS8e -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On 15 September 2014 21:39, Ian Booth wrote: > On 16/09/14 00:50, Eric Snow wrote: >> On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow wrote: >>> Yeah, those steps are a lot, though keep in mind that effectively it's >>> only 2 steps more than before if you use the -p flag to rbt post and >>> were already keeping your local master up to date. >> >> Just to be clear, here are the steps again, slightly reformatted: >> >> (0). Rebase relative to upstream master. >> - if origin is different than upstream, sync and push it > > Before I create a new branch, I ensure my local and origin (forked copy) > master > branches are up to date. However, once the branch is created, thereafter I do > not rebase because it has caused nothing but trouble, with lots of manual > effort > required to fix things up wrt conflicts etc. Here's a trick I use (I imagine it's a well known technique) that makes it easy to do a lightweight rebase without the conflicts that often come when rebase replays your commits: $ git fetch upstream $ git merge upstream/master $ git reset upstream/master $ git add any new files added in your branch $ git commit -am 'describe your branch' As far as I can make out, as long as you want to propose your branch with only a single commit added to the log, this makes it easy to use a merge-based flow and amounts to the same thing in the end. I use it a lot, and it's saved no end of "this is the *third* time I've resolved these conflicts" pain. cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On 16 September 2014 08:39, Ian Booth wrote: > > > On 16/09/14 00:50, Eric Snow wrote: > > > > Step (0) is also pretty easy and I'll argue that people should be > > doing it anyway. > > > > Disagree :-) > I never (or very, very rarely) had to do this with Launchpad and bzr and > things > Just Worked. I don't do it now with github and pull requests. I'd like to > think > we'd be able to avoid the burden moving forward also. > Sorry, I didn't mean for this to turn into a rebase vs merge discussion. A different choice of wording would have helped. The first step could have been written like: 0. Sync up your feature branch with upstream (by merging or rebasing) Some people like rebasing and some like merging. It doesn't matter much to the rest of the team which approach you use but I presume that everyone syncs up their branch somehow soon before proposing (rerunning tests etc) to ensure that other people's changes haven't impacted theirs. - Menno -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
> > FWIW, I have the following in $GOPATH/src/github.com/juju/juju/.git/config > : > > [remote "origin"] > url = g...@github.com:ericsnowcurrently/juju.git > fetch = +refs/heads/*:refs/remotes/origin/* > [remote "upstream"] > url = https://github.com/juju/juju.git > fetch = +refs/heads/*:refs/remotes/upstream/* > I have this too. I'm pretty sure this was encouraged when we switched to Github. Has anyone not got origin and upstream set up like this? Another way to check is to run: git remote -v | grep -e upstream -e origin > and in ~/.reviewboardrc: > > TRACKING_BRANCH = "upstream/master" > > This has worked fine for me (once I realized master had to be up-to-date). > This sorts out the issue for me too, avoiding the need to worry about keeping your personal master branch on GH updated. If we establish that everyone has upstream set up as above then we should update the checked-in .reviewboardrc files for each repo. - Menno -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
Is there a setting to make reviewboard show the diff in a review by the default ? For some reason for me it always requires me to hit the 'view diff' link. On Tue, Sep 16, 2014 at 6:39 AM, Ian Booth wrote: > > > On 16/09/14 00:50, Eric Snow wrote: >> On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow wrote: >>> Yeah, those steps are a lot, though keep in mind that effectively it's >>> only 2 steps more than before if you use the -p flag to rbt post and >>> were already keeping your local master up to date. >> >> Just to be clear, here are the steps again, slightly reformatted: >> >> (0). Rebase relative to upstream master. >> - if origin is different than upstream, sync and push it > > Before I create a new branch, I ensure my local and origin (forked copy) > master > branches are up to date. However, once the branch is created, thereafter I do > not rebase because it has caused nothing but trouble, with lots of manual > effort > required to fix things up wrt conflicts etc. And it should not be necessary - > the code repository should be able to track the state of play such that when > you > request a merge, it knows how to create the correct diff against the current > official trunk which will be merged into. > >> >> Step (0) is also pretty easy and I'll argue that people should be >> doing it anyway. >> > > Disagree :-) > I never (or very, very rarely) had to do this with Launchpad and bzr and > things > Just Worked. I don't do it now with github and pull requests. I'd like to > think > we'd be able to avoid the burden moving forward also. > > -- > 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: ReviewBoard is now the official review tool for juju
On 16/09/14 00:50, Eric Snow wrote: > On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow wrote: >> Yeah, those steps are a lot, though keep in mind that effectively it's >> only 2 steps more than before if you use the -p flag to rbt post and >> were already keeping your local master up to date. > > Just to be clear, here are the steps again, slightly reformatted: > > (0). Rebase relative to upstream master. > - if origin is different than upstream, sync and push it Before I create a new branch, I ensure my local and origin (forked copy) master branches are up to date. However, once the branch is created, thereafter I do not rebase because it has caused nothing but trouble, with lots of manual effort required to fix things up wrt conflicts etc. And it should not be necessary - the code repository should be able to track the state of play such that when you request a merge, it knows how to create the correct diff against the current official trunk which will be merged into. > > Step (0) is also pretty easy and I'll argue that people should be > doing it anyway. > Disagree :-) I never (or very, very rarely) had to do this with Launchpad and bzr and things Just Worked. I don't do it now with github and pull requests. I'd like to think we'd be able to avoid the burden moving forward also. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On Mon, Sep 15, 2014 at 12:54 PM, Dimiter Naydenov wrote: > - From my meager experience with writing git plugins (any executable in > $PATH with "git-" prefix), what are you describing can be easily > achieved. If you write a git plugin, named e.g. "git-rbpropose", using > the GitHub CLI (https://github.com/github/hub) and rbt, you can > automate the process: > 1. Pushing the branch to origin (checking for uncommitted changes). > 2. Creating a GitHub PR for the branch, which includes launching the > default editor to fill-in the PR description (using hub). > 3. Creating a RB review (using rbt). > 4. Optionally opening the default browser with it. > > I have a couple of handy scripts that do this, which I've shared before: > git-sync (), and git-propose (), along with a few aliases (). git-sync > fits especially well with the RB workflow, because it pull > upstream/master into your local repo's master, then pushes it back to > origin/master, and finally (when you're on a branch other than master) > rebases all branch commits over origin/master, interactively. What > usually do is run "git propose" after the finaly "git sync" to create > a PR - the only thing missing is the RB steps. Cool. I'll take a look. > > Cheers and thanks for all the hard work around putting RB workflow in > place, Glad to do it. :) -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Yeah, sorry I forgot the pastebin links to the scripts: git-sync: http://paste.ubuntu.com/8352455/ git-propose: http://paste.ubuntu.com/8352460/ git config -l | grep alias: http://paste.ubuntu.com/8352470/ (useful aliases I use) On 15.09.2014 21:54, Dimiter Naydenov wrote: > Hi Eric, > > On 15.09.2014 21:18, Eric Snow wrote: >> On Mon, Sep 15, 2014 at 11:05 AM, Katherine Cox-Buday >> wrote: >>> Let me preface this by saying I like the Reviewboard style of >>> reviewing changes. >>> >>> It's somewhat concerning to me that our reviews are now >>> disconnected from what will actually be landed into trunk. In >>> Github, you were reviewing the actual diff which would be >>> landed. In reviewboard, we're now reviewing a diff manually >>> uploaded by the developer. There's a lot of room for error in >>> terms of what diff we review vs. what diff we land. >>> >>> Any thoughts on how to couple these things once again? > >> I'm working on integration between github and reviewboard such >> that creation of a PR creates a new review request >> automatically. The same goes for updating a PR. Not only will >> that address what you are talking about, it will remove the extra >> steps of creating/updating the review request yourself and of >> closing a review request as submitted. Ergo, rbt would not be >> needed for the typical workflow. You would still use it for >> "pipedlined" branches, though that could probably be automated as >> well. > > - From my meager experience with writing git plugins (any > executable in $PATH with "git-" prefix), what are you describing > can be easily achieved. If you write a git plugin, named e.g. > "git-rbpropose", using the GitHub CLI > (https://github.com/github/hub) and rbt, you can automate the > process: 1. Pushing the branch to origin (checking for uncommitted > changes). 2. Creating a GitHub PR for the branch, which includes > launching the default editor to fill-in the PR description (using > hub). 3. Creating a RB review (using rbt). 4. Optionally opening > the default browser with it. > > I have a couple of handy scripts that do this, which I've shared > before: git-sync (), and git-propose (), along with a few aliases > (). git-sync fits especially well with the RB workflow, because it > pull upstream/master into your local repo's master, then pushes it > back to origin/master, and finally (when you're on a branch other > than master) rebases all branch commits over origin/master, > interactively. What usually do is run "git propose" after the > finaly "git sync" to create a PR - the only thing missing is the RB > steps. > >> There is the possibility of pushing info from ReviewBoard back to >> github (e.g. "ship-it" -> "LGTM" comment), but I don't think it >> buys us enough to make it worth it (it's notably trickier). > >> -eric > > > Cheers and thanks for all the hard work around putting RB workflow > in place, > - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUFzccAAoJENzxV2TbLzHwKkgH/0f4/oc2uiuo/vgbvjVM0UWe /0cF7MshfJV1dVRFZjBJV8EKuKNM0Z3DIJ6ypljJTRqn+osd3pv7svheqTD5ZE+k JGjYZJ2HuZDgxeL/0yXLT+dWjKj/5dyZjTRKmQacASXMhO3siEoMVhK1kYoTDRNp 4lQtYWkxaLmlCxIobRWaGDQT2TwQ0ICIgicwXXYBqaAa197Gkbc/vbCOpVHJyxHM HvKut28qen1HlWat9lvUcyrnA0337398p+f7gXWam/5Co9WIWiqtAw+At5ay2AIk 5n/0orPB9sGRqQzV8sYXTDosxkNe3WLFyoLkW2iMy1tnXrGBV3xuKq2yGke6+1c= =bxal -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi Eric, On 15.09.2014 21:18, Eric Snow wrote: > On Mon, Sep 15, 2014 at 11:05 AM, Katherine Cox-Buday > wrote: >> Let me preface this by saying I like the Reviewboard style of >> reviewing changes. >> >> It's somewhat concerning to me that our reviews are now >> disconnected from what will actually be landed into trunk. In >> Github, you were reviewing the actual diff which would be landed. >> In reviewboard, we're now reviewing a diff manually uploaded by >> the developer. There's a lot of room for error in terms of what >> diff we review vs. what diff we land. >> >> Any thoughts on how to couple these things once again? > > I'm working on integration between github and reviewboard such > that creation of a PR creates a new review request automatically. > The same goes for updating a PR. Not only will that address what > you are talking about, it will remove the extra steps of > creating/updating the review request yourself and of closing a > review request as submitted. Ergo, rbt would not be needed for the > typical workflow. You would still use it for "pipedlined" > branches, though that could probably be automated as well. - From my meager experience with writing git plugins (any executable in $PATH with "git-" prefix), what are you describing can be easily achieved. If you write a git plugin, named e.g. "git-rbpropose", using the GitHub CLI (https://github.com/github/hub) and rbt, you can automate the process: 1. Pushing the branch to origin (checking for uncommitted changes). 2. Creating a GitHub PR for the branch, which includes launching the default editor to fill-in the PR description (using hub). 3. Creating a RB review (using rbt). 4. Optionally opening the default browser with it. I have a couple of handy scripts that do this, which I've shared before: git-sync (), and git-propose (), along with a few aliases (). git-sync fits especially well with the RB workflow, because it pull upstream/master into your local repo's master, then pushes it back to origin/master, and finally (when you're on a branch other than master) rebases all branch commits over origin/master, interactively. What usually do is run "git propose" after the finaly "git sync" to create a PR - the only thing missing is the RB steps. > There is the possibility of pushing info from ReviewBoard back to > github (e.g. "ship-it" -> "LGTM" comment), but I don't think it > buys us enough to make it worth it (it's notably trickier). > > -eric > Cheers and thanks for all the hard work around putting RB workflow in place, - -- Dimiter Naydenov juju-core team -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUFzXyAAoJENzxV2TbLzHwPT8H/jeke5kS/VoNxL/mmGaK4IWW 5+tS7VNH9DewaRj4ZPsLtKmQvHW5oXYf5aJFEXpC3LFD9vqkyyNJQwoOaoBOQnwh FkDmTOALgXYYBd7PPsXI3fjUhjfKdcug8y7SmLLyvS61APHV1yH5++hyJdsHUany 80kFVkTbkXoMRW7BtdHREgH/tDdnEaHgJpQzbUPEuk/+gd82+q4+lnbvfs8sR/00 x1OGXzdiyGjwKSgbeFyEfUD0+mIG8xI7IUx1oVTLoLYM82i6WGZOB6g3qnh3s/bK 0ylyy09q645SNyDgsuZp8Bt8VrG92K057M/O/O+QnkeDkSHuqb27hKS+wFO2Ekg= =mfDJ -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
... > > There is the possibility of pushing info from ReviewBoard back to > github (e.g. "ship-it" -> "LGTM" comment), but I don't think it buys > us enough to make it worth it (it's notably trickier). > > -eric > > I would think it would be just as easy to change the bot to merge based on comments on Reviewboard rather than comments in Github. John =:-> -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On Mon, Sep 15, 2014 at 11:05 AM, Katherine Cox-Buday wrote: > Let me preface this by saying I like the Reviewboard style of reviewing > changes. > > It's somewhat concerning to me that our reviews are now disconnected from > what will actually be landed into trunk. In Github, you were reviewing the > actual diff which would be landed. In reviewboard, we're now reviewing a > diff manually uploaded by the developer. There's a lot of room for error in > terms of what diff we review vs. what diff we land. > > Any thoughts on how to couple these things once again? I'm working on integration between github and reviewboard such that creation of a PR creates a new review request automatically. The same goes for updating a PR. Not only will that address what you are talking about, it will remove the extra steps of creating/updating the review request yourself and of closing a review request as submitted. Ergo, rbt would not be needed for the typical workflow. You would still use it for "pipedlined" branches, though that could probably be automated as well. There is the possibility of pushing info from ReviewBoard back to github (e.g. "ship-it" -> "LGTM" comment), but I don't think it buys us enough to make it worth it (it's notably trickier). -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
Let me preface this by saying I like the Reviewboard style of reviewing changes. It's somewhat concerning to me that our reviews are now disconnected from what will actually be landed into trunk. In Github, you were reviewing the actual diff which would be landed. In reviewboard, we're now reviewing a diff manually uploaded by the developer. There's a lot of room for error in terms of what diff we review vs. what diff we land. Any thoughts on how to couple these things once again? - Katherine On Mon, Sep 15, 2014 at 10:03 AM, Eric Snow wrote: > On Sun, Sep 14, 2014 at 10:28 PM, Menno Smits > wrote: > > It looks like the TRACKING_BRANCH option in .reviewboardrc could be > helpful. > > It defaults to "origin/master" but if we changed it to "upstream/master" > I > > suspect Reviewboard will then generate diffs against the shared master > > branch instead of what we might happen to have in master in our personal > > forks. The of course relies on every developer having a remote called > > "upstream" that points to the right place (which isn't necessarily true). > > FWIW, I have the following in $GOPATH/src/github.com/juju/juju/.git/config > : > > [remote "origin"] > url = g...@github.com:ericsnowcurrently/juju.git > fetch = +refs/heads/*:refs/remotes/origin/* > [remote "upstream"] > url = https://github.com/juju/juju.git > fetch = +refs/heads/*:refs/remotes/upstream/* > > and in ~/.reviewboardrc: > > TRACKING_BRANCH = "upstream/master" > > This has worked fine for me (once I realized master had to be up-to-date). > > -eric > > -- > 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: ReviewBoard is now the official review tool for juju
On Sun, Sep 14, 2014 at 10:28 PM, Menno Smits wrote: > It looks like the TRACKING_BRANCH option in .reviewboardrc could be helpful. > It defaults to "origin/master" but if we changed it to "upstream/master" I > suspect Reviewboard will then generate diffs against the shared master > branch instead of what we might happen to have in master in our personal > forks. The of course relies on every developer having a remote called > "upstream" that points to the right place (which isn't necessarily true). FWIW, I have the following in $GOPATH/src/github.com/juju/juju/.git/config: [remote "origin"] url = g...@github.com:ericsnowcurrently/juju.git fetch = +refs/heads/*:refs/remotes/origin/* [remote "upstream"] url = https://github.com/juju/juju.git fetch = +refs/heads/*:refs/remotes/upstream/* and in ~/.reviewboardrc: TRACKING_BRANCH = "upstream/master" This has worked fine for me (once I realized master had to be up-to-date). -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
Really, rbt pull -p is the only new step. All the rest of that is stuff you should already be doing as a normal part of writing code and making pull requests. I guess adding the link on the PR to the review is also a new step. If you really want to count that as a step. On Mon, Sep 15, 2014 at 10:50 AM, Eric Snow wrote: > On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow > wrote: > > Yeah, those steps are a lot, though keep in mind that effectively it's > > only 2 steps more than before if you use the -p flag to rbt post and > > were already keeping your local master up to date. > > Just to be clear, here are the steps again, slightly reformatted: > > (0). Rebase relative to upstream master. > - if origin is different than upstream, sync and push it > 1. Create a pull request via github. > 2. Run "rbt pull -p" while at your branch to create a review request. > 3. add a comment to the PR with a link to the review request. > 4. address reviews until you get a "Ship It!" (like normal, with LGTM). > 5. add a $$merge$$ comment to the PR (like normal). > 6. mark the review request as submitted. > > So, steps 2, 3, and 6 are completely new. They don't add a lot of > work and I plan on automating all 3 of those new steps. > > Step (0) is also pretty easy and I'll argue that people should be > doing it anyway. > > -eric > > -- > 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: ReviewBoard is now the official review tool for juju
On Mon, Sep 15, 2014 at 8:09 AM, Eric Snow wrote: > Yeah, those steps are a lot, though keep in mind that effectively it's > only 2 steps more than before if you use the -p flag to rbt post and > were already keeping your local master up to date. Just to be clear, here are the steps again, slightly reformatted: (0). Rebase relative to upstream master. - if origin is different than upstream, sync and push it 1. Create a pull request via github. 2. Run "rbt pull -p" while at your branch to create a review request. 3. add a comment to the PR with a link to the review request. 4. address reviews until you get a "Ship It!" (like normal, with LGTM). 5. add a $$merge$$ comment to the PR (like normal). 6. mark the review request as submitted. So, steps 2, 3, and 6 are completely new. They don't add a lot of work and I plan on automating all 3 of those new steps. Step (0) is also pretty easy and I'll argue that people should be doing it anyway. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
On Sun, Sep 14, 2014 at 10:28 PM, Menno Smits wrote:
> Eric,
>
> Thanks for setting this up.
>
> Firstly, in your email you mention "rbt pull" several times. I think you
> mean "rbt post" right? I don't see a pull command documented anywhere.
Correct. I meant "rbt post". :)
>
> I've run in to one issue so far. When I tried to get my first review in to
> Reviewboard today it took me a long time to figure out how to get it to
> generate the correct diff. After much gnashing of teeth I figured out that
> "rbt post" generates a diff by comparing "origin/master" against the current
> branch. This means that if you haven't updated your local master branch
> recently and pushed your local master branch to your personal fork on Github
> (this is the part I missed) you'll end up with diffs that include lots of
> changes that have already been merged and have nothing to do with your
> branch.
>
> As things stand the workflow actually needs to be:
>
> 1. Ensure your feature branch is rebased against upstream/master
> 2. Create a pull request like normal via github.
> 3. Switch to your local master branch.
> 4. "git pull" to update master
> 5. "git push origin master" to update your personal master on github.
> 5. Switch back to your feature branch ("git checkout -" helps here)
> 6. Run "rbt post" while at your branch to create a review request.
> 7. open the review request in your browser and "publish" it.
> - alternately use the rbt --open (-o) and/or --publish (-p) flags.
> 8. add a comment to the PR with a link to the review request.
> 9. address reviews until you get a "Ship It!" (like normal, with LGTM).
> 10. add a $$merge$$ comment to the PR (like normal).
>
> This is a bit confusing and inconvenient. I can see us all forgetting to
> keep our personal master branches on GH updated.
Yeah, those steps are a lot, though keep in mind that effectively it's
only 2 steps more than before if you use the -p flag to rbt post and
were already keeping your local master up to date.
However, I'll look into a cleaner approach. If rbt cannot handle it,
it may make sense to write a quick "git review" command that wraps all
that. FYI, I've been using a "git refresh" command for a while that
wraps the pull/rebase steps, so writing one that does the extra review
steps shouldn't be too onerous.
All this is part of why I had considered waiting until we could roll
out proper github-reviewboard integration. However, I felt like
ReviewBoard was enough of a benefit on its own that waiting for the
integration was unwarranted.
>
> It looks like the TRACKING_BRANCH option in .reviewboardrc could be helpful.
> It defaults to "origin/master" but if we changed it to "upstream/master" I
> suspect Reviewboard will then generate diffs against the shared master
> branch instead of what we might happen to have in master in our personal
> forks. The of course relies on every developer having a remote called
> "upstream" that points to the right place (which isn't necessarily true).
I'll take a look at that.
>
> If TRACKING_BRANCH isn't going to work then whatever automation we come up
> with to streamline RB/GH integration is probably going to have to sort this
> out.
Ultimately I think it's worth getting the tooling right in the very
short term. :)
-eric
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
>
> ...
> As things stand the workflow actually needs to be:
>
> 1. Ensure your feature branch is rebased against upstream/master
>
Having this be step 1 seems like a serious put-off for using the tool. Does
it seriously not know how to calculate the merge ancestor? Git even
provides an explicit command "git merge-base" for this.
Having to rebase your work just to put it up for review is *terrible*. How
do you do incremental updates and shared work? (I'm of the mind that once
something is pushed/published rebasing again is just asking for pain.)
Certainly if you do chained work (branch a depends on b depends on c)
having to rebase them is also going to bring a lot of pain.
Maybe its just that you have to have pulled the latest upstream master and
merged it into your own branch, but even that is crummy. Tools have been
able to identify what changes you're bringing to the party for a long time.
I personally still like launchpad's "actually do a merge and show the diff
of the result including conflicts", but I realize a "show the diff against
the merge base" is a moderate compromise.
Having it go all the way to "take the diff of tip of this branch vs tip of
that branch" is really quite bad.
> 2. Create a pull request like normal via github.
> 3. Switch to your local master branch.
> 4. "git pull" to update master
> 5. "git push origin master" to update your personal master on github.
> 5. Switch back to your feature branch ("git checkout -" helps here)
> 6. Run "rbt post" while at your branch to create a review request.
> 7. open the review request in your browser and "publish" it.
> - alternately use the rbt --open (-o) and/or --publish (-p) flags.
> 8. add a comment to the PR with a link to the review request.
> 9. address reviews until you get a "Ship It!" (like normal, with LGTM).
> 10. add a $$merge$$ comment to the PR (like normal).
>
> This is a bit confusing and inconvenient. I can see us all forgetting to
> keep our personal master branches on GH updated.
>
> It looks like the TRACKING_BRANCH option in .reviewboardrc could be
> helpful. It defaults to "origin/master" but if we changed it to
> "upstream/master" I suspect Reviewboard will then generate diffs against
> the shared master branch instead of what we might happen to have in master
> in our personal forks. The of course relies on every developer having a
> remote called "upstream" that points to the right place (which isn't
> necessarily true).
>
> If TRACKING_BRANCH isn't going to work then whatever automation we come up
> with to streamline RB/GH integration is probably going to have to sort this
> out.
>
> - Menno
>
>
>
That does sound like an awful lot of steps for what used to be "lbox
propose" (which would push and publish and ask you for a message, and find
the right base, etc.)
John
=:->
>
>
>
>
>
>
> On 14 September 2014 14:45, Eric Snow wrote:
>
>> Hi all,
>>
>> Starting now new code review requests should be made on
>> http://reviews.vapour.ws (see more below on creating review requests).
>> We will continue to use github for everything else (pull requests,
>> merging, etc.). I've already posted some of the below information
>> elsewhere, but am repeating it here for the sake of reference. I plan
>> on updating CONTRIBUTING.md with this information in the near future.
>> Please let me know if you have any feedback. Happy reviewing!
>>
>> -eric
>>
>> Authentication
>> --
>> Use the Github OAuth button on the login page to log in. If you don't
>> have an account yet on ReviewBoard, your github account name will
>> automatically be registered for you. ReviewBoard uses session
>> cookies, so once you have logged in you shouldn't need to log in again
>> unless you log out first.
>>
>> For the reviewboard commandline client (rbt), use your reviewboard
>> username and a password of "oauth:@github". This should
>> only be needed the first time.
>>
>> RBTools
>> --
>>
>> ReviewBoard has a command-line tool that you can install on your local
>> system called "rbt". rbt is the recommended tool for creating and
>> updating review requestsion. The documentation covers installation
>> and usage. It has satisfied my questions thus far.
>>
>> https://www.reviewboard.org/docs/rbtools/0.6/
>>
>> The key sub-command is "post" (see rbt post -h).
>>
>> To install you can follow the instructions in the rbtools docs. You
>> can also install using pip (which itself may need to be installed
>> first):
>>
>> $ virtualenv ~/.venvs/reviewboard ~/.venvs/reviewboard/bin/pip install
>> --allow-unverified rbtools --allow-external rbtools rbtools
>> $ alias rbt='~/.venvs/reviewboard/bin/rbt'
>>
>> (you could just "sudo pip install" it, but the --allow-unverified flag
>> makes it kind of sketchy.)
>>
>> Workflow
>> ---
>>
>> 1. Create a pull request like normal via github.
>> 2. Run "rbt pull" while at your branch to create a review request.
>> - if the repo does not have a .reviewboardrc file yet, you'll
Re: ReviewBoard is now the official review tool for juju
Eric,
Thanks for setting this up.
Firstly, in your email you mention "rbt pull" several times. I think you
mean "rbt post" right? I don't see a pull command documented anywhere.
I've run in to one issue so far. When I tried to get my first review in to
Reviewboard today it took me a long time to figure out how to get it to
generate the correct diff. After much gnashing of teeth I figured out that
"rbt post" generates a diff by comparing "origin/master" against the
current branch. This means that if you haven't updated your local master
branch recently *and pushed your local master branch to your personal fork
on Github* (this is the part I missed) you'll end up with diffs that
include lots of changes that have already been merged and have nothing to
do with your branch.
As things stand the workflow actually needs to be:
1. Ensure your feature branch is rebased against upstream/master
2. Create a pull request like normal via github.
3. Switch to your local master branch.
4. "git pull" to update master
5. "git push origin master" to update your personal master on github.
5. Switch back to your feature branch ("git checkout -" helps here)
6. Run "rbt post" while at your branch to create a review request.
7. open the review request in your browser and "publish" it.
- alternately use the rbt --open (-o) and/or --publish (-p) flags.
8. add a comment to the PR with a link to the review request.
9. address reviews until you get a "Ship It!" (like normal, with LGTM).
10. add a $$merge$$ comment to the PR (like normal).
This is a bit confusing and inconvenient. I can see us all forgetting to
keep our personal master branches on GH updated.
It looks like the TRACKING_BRANCH option in .reviewboardrc could be
helpful. It defaults to "origin/master" but if we changed it to
"upstream/master" I suspect Reviewboard will then generate diffs against
the shared master branch instead of what we might happen to have in master
in our personal forks. The of course relies on every developer having a
remote called "upstream" that points to the right place (which isn't
necessarily true).
If TRACKING_BRANCH isn't going to work then whatever automation we come up
with to streamline RB/GH integration is probably going to have to sort this
out.
- Menno
On 14 September 2014 14:45, Eric Snow wrote:
> Hi all,
>
> Starting now new code review requests should be made on
> http://reviews.vapour.ws (see more below on creating review requests).
> We will continue to use github for everything else (pull requests,
> merging, etc.). I've already posted some of the below information
> elsewhere, but am repeating it here for the sake of reference. I plan
> on updating CONTRIBUTING.md with this information in the near future.
> Please let me know if you have any feedback. Happy reviewing!
>
> -eric
>
> Authentication
> --
> Use the Github OAuth button on the login page to log in. If you don't
> have an account yet on ReviewBoard, your github account name will
> automatically be registered for you. ReviewBoard uses session
> cookies, so once you have logged in you shouldn't need to log in again
> unless you log out first.
>
> For the reviewboard commandline client (rbt), use your reviewboard
> username and a password of "oauth:@github". This should
> only be needed the first time.
>
> RBTools
> --
>
> ReviewBoard has a command-line tool that you can install on your local
> system called "rbt". rbt is the recommended tool for creating and
> updating review requestsion. The documentation covers installation
> and usage. It has satisfied my questions thus far.
>
> https://www.reviewboard.org/docs/rbtools/0.6/
>
> The key sub-command is "post" (see rbt post -h).
>
> To install you can follow the instructions in the rbtools docs. You
> can also install using pip (which itself may need to be installed
> first):
>
> $ virtualenv ~/.venvs/reviewboard ~/.venvs/reviewboard/bin/pip install
> --allow-unverified rbtools --allow-external rbtools rbtools
> $ alias rbt='~/.venvs/reviewboard/bin/rbt'
>
> (you could just "sudo pip install" it, but the --allow-unverified flag
> makes it kind of sketchy.)
>
> Workflow
> ---
>
> 1. Create a pull request like normal via github.
> 2. Run "rbt pull" while at your branch to create a review request.
> - if the repo does not have a .reviewboardrc file yet, you'll need
> to run "rbt setup-repo".
> - make sure your branch is based on an up-to-date master.
> - if the revision already has a review request you will need to
> update it (see below).
> 3. open the review request in your browser and "publish" it.
> - alternately use the rbt --open (-o) and/or --publish (-p) flags.
> 4. add a comment to the PR with a link to the review request.
> 5. address reviews until you get a "Ship It!" (like normal, with LGTM).
> 6. add a $$merge$$ comment to the PR (like normal).
>
> Keep in mind that the github-related steps aren't strictly necessary
> for the sake a getti
Re: ReviewBoard is now the official review tool for juju
On Sun, Sep 14, 2014 at 2:59 PM, Jesse Meek wrote: > Thank you for setting this up. Is it possible to add 'unpublished' review > comments such that you can add/remove/edit comments until you've completed a > review, at which point you 'publish' all review comments, making them > visible to the author? That's how ReviewBoard already works. Everything it draft until you hit the "publish" button. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard is now the official review tool for juju
Thank you for setting this up. Is it possible to add 'unpublished' review comments such that you can add/remove/edit comments until you've completed a review, at which point you 'publish' all review comments, making them visible to the author? On 14/09/14 14:45, Eric Snow wrote: Hi all, Starting now new code review requests should be made on http://reviews.vapour.ws (see more below on creating review requests). We will continue to use github for everything else (pull requests, merging, etc.). I've already posted some of the below information elsewhere, but am repeating it here for the sake of reference. I plan on updating CONTRIBUTING.md with this information in the near future. Please let me know if you have any feedback. Happy reviewing! -eric Authentication -- Use the Github OAuth button on the login page to log in. If you don't have an account yet on ReviewBoard, your github account name will automatically be registered for you. ReviewBoard uses session cookies, so once you have logged in you shouldn't need to log in again unless you log out first. For the reviewboard commandline client (rbt), use your reviewboard username and a password of "oauth:@github". This should only be needed the first time. RBTools -- ReviewBoard has a command-line tool that you can install on your local system called "rbt". rbt is the recommended tool for creating and updating review requestsion. The documentation covers installation and usage. It has satisfied my questions thus far. https://www.reviewboard.org/docs/rbtools/0.6/ The key sub-command is "post" (see rbt post -h). To install you can follow the instructions in the rbtools docs. You can also install using pip (which itself may need to be installed first): $ virtualenv ~/.venvs/reviewboard ~/.venvs/reviewboard/bin/pip install --allow-unverified rbtools --allow-external rbtools rbtools $ alias rbt='~/.venvs/reviewboard/bin/rbt' (you could just "sudo pip install" it, but the --allow-unverified flag makes it kind of sketchy.) Workflow --- 1. Create a pull request like normal via github. 2. Run "rbt pull" while at your branch to create a review request. - if the repo does not have a .reviewboardrc file yet, you'll need to run "rbt setup-repo". - make sure your branch is based on an up-to-date master. - if the revision already has a review request you will need to update it (see below). 3. open the review request in your browser and "publish" it. - alternately use the rbt --open (-o) and/or --publish (-p) flags. 4. add a comment to the PR with a link to the review request. 5. address reviews until you get a "Ship It!" (like normal, with LGTM). 6. add a $$merge$$ comment to the PR (like normal). Keep in mind that the github-related steps aren't strictly necessary for the sake a getting a code review. They are if you want to merge the patch though. :) I mention this because sometimes you want a review on something for which you can't create a decent PR in github (see "Patch Series" below). Updates -- To update a review request use "rbt pull -u" or "rbt pull -r #". This will update the corresponding existing review request. Note: Reviewboard links revision IDs to review requests. So if you already have a review request for a particular revision (e.g. your branch), then a simple "rbt post" will fail. Patch Series - Sometimes you have one branch that depends on another, or perhaps several such forming a chain of branches. While github does not cope well with this, ReviewBoard does just fine. Use the parent flag: rbt post --parent . Workflow Automation -- Currently we do not have any integration set up between reviewboard and github. However, we plan on doing so later, at which point you won't need to create/update review requests manually. There is other automation we could target, but that can be addressed later. Email --- ReviewBoard is currently set up to send out notification emails. However, currently registered users do not have an email address set. So if you want to get review-related email, please make sure you set your account's email address in ReviewBoard. SSL --- We working out the details of precuring an SSL certificate for reviews.vapour.ws. We have a self-signed cert, but that isn't a long-term solution and makes browsers fussy, so we are going to wait on a CA-signed cert. Once we switch over the URL will change to https://reviews.vapour.ws. However, that should be relatively transparent because reviewboard will automatically redirect to https at that point. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
On Thu, Sep 11, 2014 at 9:10 AM, Frank Mueller wrote: > Oh, I have to thank you. Being focussed on the doc directory I simply fogot > the standard CONTRIBUTING file. Maybe, if it grows more and more, it is > worth to see this document as a central and quick entry point with > references into the specific and detailed documents in doc/contributions. > > So let's start with your idea, I'll see how the doc grows and when it makes > sense to distribute the content into individual files. Sounds good. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
Hi Eric, I was planning on updating > https://github.com/juju/juju/blob/master/CONTRIBUTING.md once I felt > comfortable with how reviewboard fit into our workflow. Do you think > something in juju/doc/contributions, which perhaps elaborates on the > info in CONTRIBUTING.md, would be a worthy addition? Thanks for > bringing this up, by the way. :) > Oh, I have to thank you. Being focussed on the doc directory I simply fogot the standard CONTRIBUTING file. Maybe, if it grows more and more, it is worth to see this document as a central and quick entry point with references into the specific and detailed documents in doc/contributions. So let's start with your idea, I'll see how the doc grows and when it makes sense to distribute the content into individual files. thx mue -- ** Frank Mueller ** Software Engineer - Juju Development ** Canonical -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
On Thu, Sep 11, 2014 at 1:34 AM, Frank Mueller
wrote:
> For switching to a new tool and a new workflow I would like to not simply
> discuss it in a somehow undefined way together with subjunctive terms
> ("Everybody should now ...") here via mail. Please lets fix the workflow in
> a how-to-contribute.md in juju/doc/contributions, so that we easily can
> point any new internal and external contributor to it. And also let's have a
> well defined date for switching and communicate it early enough.
>
> If somebode is already writing this doc mentioned above please let me know.
> Otherwise I'll do it.
Hi Frank,
I was planning on updating
https://github.com/juju/juju/blob/master/CONTRIBUTING.md once I felt
comfortable with how reviewboard fit into our workflow. Do you think
something in juju/doc/contributions, which perhaps elaborates on the
info in CONTRIBUTING.md, would be a worthy addition? Thanks for
bringing this up, by the way. :)
-eric
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
For switching to a new tool and a new workflow I would like to not simply
discuss it in a somehow undefined way together with subjunctive terms
("Everybody should now ...") here via mail. Please lets fix the workflow in
a how-to-contribute.md in juju/doc/contributions, so that we easily can
point any new internal and external contributor to it. And also let's have
a well defined date for switching and communicate it early enough.
If somebode is already writing this doc mentioned above please let me know.
Otherwise I'll do it.
thx mue
On Wed, Sep 10, 2014 at 11:47 PM, Eric Snow wrote:
> On Wed, Sep 10, 2014 at 3:34 PM, Menno Smits
> wrote:
> > Thanks Eric! I've used Reviewboard at a previous job and I'm fairly sure
> > that it aligns better with the way the Juju Core team likes to work than
> > Github's review features.
> >
> > Two questions:
> >
> > 1. Is this what we're supposed to be doing from right now?
>
> Nope. I'm hopeful that we will switch over to reviewboard in a week
> or two. Until then github is still our review tool. However, in the
> meantime feel free to try out the Reviewboard-oriented workflow as
> well. :)
>
> >
> > 2. I'm pretty sure some configuration of the rbt tool is required so
> that it
> > knows how to talk to the Reviewboard server. Is there a config file
> > available?
>
> The first time in a repo you run "rbt setup-repo" which generates a
> user-agnostic, repo-specific .reviewboardrc file. I expect that
> before long we will commit that file in each of our repos. Once that
> happens, no one will have to do anything special (e.g. run rbt
> setup-repo) any longer.
>
> See https://www.reviewboard.org/docs/rbtools/dev/rbt/configuration/
>
> -eric
>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
--
** Frank Mueller
** Software Engineer - Juju Development
** Canonical
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
On Wed, Sep 10, 2014 at 3:34 PM, Menno Smits wrote: > Thanks Eric! I've used Reviewboard at a previous job and I'm fairly sure > that it aligns better with the way the Juju Core team likes to work than > Github's review features. > > Two questions: > > 1. Is this what we're supposed to be doing from right now? Nope. I'm hopeful that we will switch over to reviewboard in a week or two. Until then github is still our review tool. However, in the meantime feel free to try out the Reviewboard-oriented workflow as well. :) > > 2. I'm pretty sure some configuration of the rbt tool is required so that it > knows how to talk to the Reviewboard server. Is there a config file > available? The first time in a repo you run "rbt setup-repo" which generates a user-agnostic, repo-specific .reviewboardrc file. I expect that before long we will commit that file in each of our repos. Once that happens, no one will have to do anything special (e.g. run rbt setup-repo) any longer. See https://www.reviewboard.org/docs/rbtools/dev/rbt/configuration/ -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: ReviewBoard and our workflow
Thanks Eric! I've used Reviewboard at a previous job and I'm fairly sure that it aligns better with the way the Juju Core team likes to work than Github's review features. Two questions: 1. Is this what we're supposed to be doing from right now? 2. I'm pretty sure some configuration of the rbt tool is required so that it knows how to talk to the Reviewboard server. Is there a config file available? On 11 September 2014 03:58, Eric Snow wrote: > Steps for a review of a PR: > > 1. create pull request in github > 2. run "rbt post" while at your branch to create a review request [1][2] > 3. open the review request in your browser and "publish" it [3] > 4. add a comment to the PR with a link to the review request > 5. address reviews until you get a "Ship It!" > 6. add a $$merge$$ comment to the PR > > Both github and ReviewBoard support various triggers/hooks and both > have robust HTTP APIs. So we should be able to automate those steps > (e.g. PR -> review request, "ship it" -> $$merge$$). However, I don't > see that automation as a prerequisite for switching over to > ReviewBoard. > > Updating an existing review request: > 1. run "rbt post -u" (or the explicit "rbt post -r #") > 2. open the review request in your browser and "publish" it [3] > > FYI, Reviewboard supports chaining review requests. Run rbt post > --parent . > > I'll be updating the contributing doc relative to ReviewBoard (i.e. > with the above info) once we settle in with the new tool. > > -eric > > [1] Make sure your branch is based on upstream master. Otherwise this > will not work right. > [2] Reviewboard links revision IDs to review requests. So if you > already have a review request for a particular revision (e.g. your > branch), then "rbt post" will fail. Use "rbt post -u" or "rbt post -r > #" instead. > [3] rbt post has some options you should consider using: > - automatically publish the review request: rbt post --publish (or -p) > - open a browser window with the new review request: rbt post --open (or > -o) > > -- > 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: reviewboard
On 10/09/14 02:47, Eric Snow wrote: > On Mon, Sep 8, 2014 at 8:05 PM, Tim Penhey wrote: >> On 09/09/14 04:32, Eric Snow wrote: >>> To install rbt: >>> >>> sudo pip install --allow-unverified rbtools --allow-external rbtools rbtools >> >> Ah... no. >> >> Perhaps in a virtual env. > > Is it the sudo or the flags to which you object? :) Using a > virtualenv would indeed be a good idea! So, as a correction to my > previous instructions: It is the combination of "sudo", "pip", and "allow-unverified". All my python packages that are not installed by system debs are in virtual environments. Tim -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
Thanks Eric, Taking it for a spin now, looks pretty cool Matty On Tue, Sep 9, 2014 at 3:47 PM, Eric Snow wrote: > On Mon, Sep 8, 2014 at 8:05 PM, Tim Penhey > wrote: > > On 09/09/14 04:32, Eric Snow wrote: > >> To install rbt: > >> > >> sudo pip install --allow-unverified rbtools --allow-external rbtools > rbtools > > > > Ah... no. > > > > Perhaps in a virtual env. > > Is it the sudo or the flags to which you object? :) Using a > virtualenv would indeed be a good idea! So, as a correction to my > previous instructions: > > virtualenv ~/.venvs/reviewboard > ~/.venvs/reviewboard/bin/pip install --allow-unverified rbtools > --allow-external rbtools rbtools > alias rbt='~/.venvs/reviewboard/bin/rbt' > > -eric > > -- > 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: reviewboard
On Mon, Sep 8, 2014 at 8:05 PM, Tim Penhey wrote: > On 09/09/14 04:32, Eric Snow wrote: >> To install rbt: >> >> sudo pip install --allow-unverified rbtools --allow-external rbtools rbtools > > Ah... no. > > Perhaps in a virtual env. Is it the sudo or the flags to which you object? :) Using a virtualenv would indeed be a good idea! So, as a correction to my previous instructions: virtualenv ~/.venvs/reviewboard ~/.venvs/reviewboard/bin/pip install --allow-unverified rbtools --allow-external rbtools rbtools alias rbt='~/.venvs/reviewboard/bin/rbt' -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
On 09/09/14 04:32, Eric Snow wrote: > On Mon, Sep 8, 2014 at 10:21 AM, Eric Snow wrote: >> In the meantime I recommend using rbt (as echoed by Adam). I know >> As a bonus, rbt allows you to create review requests relative to a >> parent revision (ergo branch), so you can chain patches. > > To install rbt: > > sudo pip install --allow-unverified rbtools --allow-external rbtools rbtools Ah... no. Perhaps in a virtual env. Tim -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
On Mon, Sep 8, 2014 at 10:23 AM, Eric Snow wrote: > Wayne just pointed out to me that rbt + our OAuth-based users aren't a > great mix. This is because the auto-registered reviewboard users are > not set up with passwords. I'll figure out a solution for us. The OAuth stuff should work fine if you use your github username for the username and "oauth:@github" for your password (replacing with your github username). -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
On Mon, Sep 8, 2014 at 10:21 AM, Eric Snow wrote: > In the meantime I recommend using rbt (as echoed by Adam). I know > As a bonus, rbt allows you to create review requests relative to a > parent revision (ergo branch), so you can chain patches. To install rbt: sudo pip install --allow-unverified rbtools --allow-external rbtools rbtools -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
On Mon, Sep 8, 2014 at 10:21 AM, Eric Snow wrote: > In the meantime I recommend using rbt (as echoed by Adam). I know > As a bonus, rbt allows you to create review requests relative to a > parent revision (ergo branch), so you can chain patches. Wayne just pointed out to me that rbt + our OAuth-based users aren't a great mix. This is because the auto-registered reviewboard users are not set up with passwords. I'll figure out a solution for us. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
On Mon, Sep 8, 2014 at 4:37 AM, Ian Booth wrote:
> Hi Eric
>
> Fantastic, thank you.
>
> Quick question - can we set up a Juju team group and have that group
> automatically be assigned as a reviewer for newly created review requests? I
> tried to create a new request using the web ui and had to manually enter the
> reviewer. Except there was no group set up yet for the Juju team.
I've added a review group ("juju-team") and a default reviewer
("juju-tream") with that group (and all repos) assigned to it. As we
add repos and users I'll make sure that stays up to date.
>
> Also, when creating a new review request, it shows a list of commits to choose
> from, whereas I would be wanting to see a list of branches since that's how we
> create the PRs on Github and the branch is what the review is based on. Can we
> fix this?
Unfortunately reviewboard doesn't support github PRs and they don't
plan on supporting it. At some point I'd like to add support for
automatically generating review requests from pull requests and
automatically notifying the CI bot when a review request gets a
ship-it.
In the meantime I recommend using rbt (as echoed by Adam). I know
As a bonus, rbt allows you to create review requests relative to a
parent revision (ergo branch), so you can chain patches.
-eric
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
Also, I've written up all the steps for deploying, as well as config changes, a to-do list, and other reviewboard-related details: https://docs.google.com/a/canonical.com/spreadsheets/d/1OLg-AEGHasnDgUUaK5mVhK8FOyvsPPJrWds6JtUhZb8/edit?usp=sharing Not all the config settings I've listed have been done yet. I'll roll those out when I can. Also note that for some of the config settings I'm using values other than what I've listed in the spreadsheet. Each of those will be changed to the correct value before we switch over. -eric On Fri, Sep 5, 2014 at 7:09 PM, Eric Snow wrote: > I'm pleased to announce that we now have a working demo of ReviewBoard > available: > > http://reviews.vapour.ws/ > > Feel free to take it for a spin! > > There's no need to register for an account. Just click the github > OAuth button on the login page. The first time you do so you'll be > redirected to a github page asking if you approve. Then you'll be > redirected back to ReviewBoard. At that point it will automatically > create an account for you and log you in. Due to session cookies, you > shouldn't need to log in all that often. > > I've set up two of the repos to start us off. I can set up more if it > would help, but the two should be enough to get us a feel for things. > > Let me know any feedback you have. There are a number of > configuration options we can tweak. > > You can do just about everything through the web interface, but I > recommend using the reviewboard client CLI "rbt" (pip install > RBTools). > > Keep in mind that this is only a demo. Though it's mostly complete > and the final release will be at the same URL (except SSL-only), > there are still settings we need to tweak and workflow considerations > to resolve. > > Consequently, the site will probably be wiped at least once before any > final release. So try it out but don't do anything important on > there. > > -eric > > p.s. Dog food was served. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: reviewboard
Thanks for responding on this, Adam. -eric On Mon, Sep 8, 2014 at 6:17 AM, Adam Collard wrote: > Hi Ian, > > On 8 September 2014 11:37, Ian Booth wrote: >> >> Hi Eric >> >> Fantastic, thank you. >> >> Quick question - can we set up a Juju team group and have that group >> automatically be assigned as a reviewer for newly created review requests? >> I >> tried to create a new request using the web ui and had to manually enter >> the >> reviewer. Except there was no group set up yet for the Juju team. > > > Yes. See > https://www.reviewboard.org/docs/manual/2.0/admin/configuration/default-reviewers/ > >> >> >> Also, when creating a new review request, it shows a list of commits to >> choose >> from, whereas I would be wanting to see a list of branches since that's >> how we >> create the PRs on Github and the branch is what the review is based on. >> Can we >> fix this? > > > It's *strongly* recommended by upstream to use the command line tool, rbt > (which Eric notes below). Docs available at > https://www.reviewboard.org/docs/rbtools/0.6/. Specifically you will want to > look at > https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems > > Adam > >> >> >> >> On 06/09/14 11:09, Eric Snow wrote: >> > I'm pleased to announce that we now have a working demo of ReviewBoard >> > available: >> > >> > http://reviews.vapour.ws/ >> > >> > Feel free to take it for a spin! >> > >> > There's no need to register for an account. Just click the github >> > OAuth button on the login page. The first time you do so you'll be >> > redirected to a github page asking if you approve. Then you'll be >> > redirected back to ReviewBoard. At that point it will automatically >> > create an account for you and log you in. Due to session cookies, you >> > shouldn't need to log in all that often. >> > >> > I've set up two of the repos to start us off. I can set up more if it >> > would help, but the two should be enough to get us a feel for things. >> > >> > Let me know any feedback you have. There are a number of >> > configuration options we can tweak. >> > >> > You can do just about everything through the web interface, but I >> > recommend using the reviewboard client CLI "rbt" (pip install >> > RBTools). >> > >> > Keep in mind that this is only a demo. Though it's mostly complete >> > and the final release will be at the same URL (except SSL-only), >> > there are still settings we need to tweak and workflow considerations >> > to resolve. >> > >> > Consequently, the site will probably be wiped at least once before any >> > final release. So try it out but don't do anything important on >> > there. >> > >> > -eric >> > >> > p.s. Dog food was served. >> > >> >> -- >> 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: reviewboard
Hi Ian, On 8 September 2014 11:37, Ian Booth wrote: > Hi Eric > > Fantastic, thank you. > > Quick question - can we set up a Juju team group and have that group > automatically be assigned as a reviewer for newly created review requests? > I > tried to create a new request using the web ui and had to manually enter > the > reviewer. Except there was no group set up yet for the Juju team. > Yes. See https://www.reviewboard.org/docs/manual/2.0/admin/configuration/default-reviewers/ > > Also, when creating a new review request, it shows a list of commits to > choose > from, whereas I would be wanting to see a list of branches since that's > how we > create the PRs on Github and the branch is what the review is based on. > Can we > fix this? > It's *strongly* recommended by upstream to use the command line tool, rbt (which Eric notes below). Docs available at https://www.reviewboard.org/docs/rbtools/0.6/. Specifically you will want to look at https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems Adam > > > On 06/09/14 11:09, Eric Snow wrote: > > I'm pleased to announce that we now have a working demo of ReviewBoard > > available: > > > > http://reviews.vapour.ws/ > > > > Feel free to take it for a spin! > > > > There's no need to register for an account. Just click the github > > OAuth button on the login page. The first time you do so you'll be > > redirected to a github page asking if you approve. Then you'll be > > redirected back to ReviewBoard. At that point it will automatically > > create an account for you and log you in. Due to session cookies, you > > shouldn't need to log in all that often. > > > > I've set up two of the repos to start us off. I can set up more if it > > would help, but the two should be enough to get us a feel for things. > > > > Let me know any feedback you have. There are a number of > > configuration options we can tweak. > > > > You can do just about everything through the web interface, but I > > recommend using the reviewboard client CLI "rbt" (pip install > > RBTools). > > > > Keep in mind that this is only a demo. Though it's mostly complete > > and the final release will be at the same URL (except SSL-only), > > there are still settings we need to tweak and workflow considerations > > to resolve. > > > > Consequently, the site will probably be wiped at least once before any > > final release. So try it out but don't do anything important on > > there. > > > > -eric > > > > p.s. Dog food was served. > > > > -- > 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: reviewboard
Hi Eric Fantastic, thank you. Quick question - can we set up a Juju team group and have that group automatically be assigned as a reviewer for newly created review requests? I tried to create a new request using the web ui and had to manually enter the reviewer. Except there was no group set up yet for the Juju team. Also, when creating a new review request, it shows a list of commits to choose from, whereas I would be wanting to see a list of branches since that's how we create the PRs on Github and the branch is what the review is based on. Can we fix this? On 06/09/14 11:09, Eric Snow wrote: > I'm pleased to announce that we now have a working demo of ReviewBoard > available: > > http://reviews.vapour.ws/ > > Feel free to take it for a spin! > > There's no need to register for an account. Just click the github > OAuth button on the login page. The first time you do so you'll be > redirected to a github page asking if you approve. Then you'll be > redirected back to ReviewBoard. At that point it will automatically > create an account for you and log you in. Due to session cookies, you > shouldn't need to log in all that often. > > I've set up two of the repos to start us off. I can set up more if it > would help, but the two should be enough to get us a feel for things. > > Let me know any feedback you have. There are a number of > configuration options we can tweak. > > You can do just about everything through the web interface, but I > recommend using the reviewboard client CLI "rbt" (pip install > RBTools). > > Keep in mind that this is only a demo. Though it's mostly complete > and the final release will be at the same URL (except SSL-only), > there are still settings we need to tweak and workflow considerations > to resolve. > > Consequently, the site will probably be wiped at least once before any > final release. So try it out but don't do anything important on > there. > > -eric > > p.s. Dog food was served. > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Reviewboard
Sorry, more context for those who haven't been in on the talks in Juju-core: We're trying to get Reviewboard set up for juju-core use with github (and a plugin we wrote so we can log in with our github usernames). On Thu, Aug 14, 2014 at 10:27 AM, Nate Finch wrote: > What's the status on this? I think this could really help our workflow. > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
