ReviewBoard is now the official review tool for juju

[Menno Smits]

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 <eric.snow at canonical.com> 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:<username>@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 <parent branch>.
>
> 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 at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140915/e5d72442/attachment-0001.html>