Meeting Minutes for October 28th, 2012


  1. Midterm evaluations have gone out, and you should have received them by email. If you haven’t, please let Mike C. know ASAP.
  2. We’re aiming for 1.7 RC this week, and a full release early November. As such we are limiting what goes into master during this time in order to maintain stability. Much of what’s being written this semester will target a 1.7.x release.

Questions / Answers / Tips:

Q (Tina): What’s the best way to commit fixes to issues on review requests? Is one WIP review request okay (to maintain context), or if it gets too big should we have multiple ones with different branches like : “main-project/sub-task”?

A: For a fix within one change, just amend that change, rather than have a new commit. Save new commits for whole pieces of work. In the end we just apply a patch so it won’t matter much for us. Breaking up a project into smaller, atomic pieces can be good and make reviewing go faster. One way or another, remove “WIP” when a change is ready for committing.

Regarding branching: you should group review requests and branches into logical pieces of work (tasks). If a task is done, create a new branch for the next one, and use parent diffs if you need to for RB.

Q (Wesley): How to get access to BuildBot in order to test ReviewBot plugins?

A: You should set up your own BuildBot. The one we have is very specialized. We’re not in a place to be able to grant access, and we don’t have sandboxing anymore.

Q (Sampson): Is my next project confirmed as “ReviewUI integration with extensions infrastructure”?

A: Yes, let’s start on that. It won’t be a very large one, but we can go from there on some features.

Q (Michelle): Are file attachments attached to a review request in general or to a specific diff?

A: Right now, just in general. With Tina’s work (this semester), both.

Q (Michelle): Should we be diffing file attachments that differ between diffs? I recall a few weeks ago you had said that we should be diffing between file attachments with the same name, but you can’t attach file attachments with the same name to the review request in general.

A: There is no infrastructure for that today. It would have to be written at some point. Right now the focus is on display/review for individual files, and on image diffing, which will both be super useful and serve as a good test for the file attachment diffing work.

Q (Karl): Will the 1.7RC change the way we work in the short term? Do we just pull from master as usual?

A: There should be no effect; just keep pulling from master.

Q (Jesus): A server error message appears and disappears when the discard review request button is clicked. The message quickly goes away and the page renders fine. Any suggestions? (NB: Error is visible in server console but disappears as well. Browser is Firefox.)

A: Try reproducing in Chrome. First open up the Network Log in the Developer Tools and click the black circle button in the bottom. Reproduce it. It should keep the HTTP request for that around, and from there, you should be able to look at the result.

Note: Developer console in Firefox will also log and report HTTP requests.

Q (John): Anyone have tips on how to force (require) cmd args for an rb command? Right now the command would be called like this: rb patch -r <request_id> [–diff-revision=<diff_revision>]. We want to call the command like this: rb patch <request_id> [<diff_revision>].

A: Look at After you parse options, you should look at the remaining argument list and ensure you have what you need. Instead of sys.argv, you’re going to access the args coming from parse_args. Those will be the remaining arguments after all –flags have been considered.

Q (Allyshia): If we want to review other peoples’ code, we just head on over to the “reviewboard” group at, pick off a review request that appears to be from one of us and start commenting?

A: It doesn’t even have to be from one of the other students. But otherwise, yes.*

* See tips on reviewing below.

Q (Allyshia): Will the 1.7 RC changes affect those working on ReviewBot in any way?

A: They shouldn’t, no.

General Tips:

Regarding reviewing:

  • We’re not expecting you to become experts on the stuff that you’re reviewing in order to find defects – just take a look, and if you see something odd, bring it up. If something is not clear, ask about it. Style problems are valid issues too.
  • Alternatively, you don’t just need to find defects. If you see a particularly “beautiful” piece of code that you think is awesome-graceful, point it out and tell the author that it’s cool!

Regarding mid-term evaluations:

  • Remember that the midterm evaluation is more of a sign-post, and is not being used for your final grade. It’s more of a way to let you know how you’re doing, though final grades will be computed in a similar manner. Feel free to approach mentors if you have any questions.

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )


Connecting to %s