Add a mach rbt command to run ReviewBoard Tools

RESOLVED FIXED in Firefox 28

Status

RESOLVED FIXED
5 years ago
7 months ago

People

(Reporter: mconley, Assigned: gps)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla28
x86
All
Dependency tree / graph

Firefox Tracking Flags

(firefox28 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
With our fancy new Review Board instance up at https://reviewboard.allizom.org/dashboard/, it'd be really excellent if we could grease the wheels and get people to try it.

One idea that's been brewing is to make it easy for developers to upload patches to Review Board via RBTools, and then to simultaneously post a link to the View Diff portion of the RB review request into the bug that the patch belongs to.

The end-result is that a reviewer need only click on an attachment in Bugzilla, and then be quickly redirected to the review request where they can immediately start reviewing.

I think a mach command making use of the RBTools WebAPI client might be a nice solution.
(Assignee)

Comment 1

5 years ago
I was thinking about this as well.

I think it should be sufficient to expose rbt directly to the end-user: no need to write a one-off mach command that simply does what `rbt post` does already.

I could go into a deep discussion about virtualenvs and shell development environments and how we should do this properly. But I won't. I think a simple `mach rbt` that installs RBTools from pip and then invokes its main() should be sufficient for now.
So does `rbt post` upload the link + review request to bmo?
(Assignee)

Comment 3

5 years ago
Yeah, I missed the 2nd half of the summary. Perhaps this should be marked as a dupe of bug 774150. That's still softly blocked on having a sane Python API to Bugzilla IIRC. A quick hack would be to extract bzexport's bugzilla code into a shared library (or just copy the code). We can install pip packages via mach commands - so shared library is preferred.

Comment 4

5 years ago
There's a fairly straightforward REST API for adding attachments; is this not sufficient?

http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Bug.html#add_attachment
bzexport uses BzAPI, which is also fairly straightforward, but it turns out there's a lot of things to get right in order to make it useful.
(Assignee)

Comment 6

5 years ago
Created attachment 8343544 [details] [diff] [review]
mach rbt command to run ReviewBoard Tools

While this patch doesn't accomplish what the bug summary requests, it
does solve a problem: making RBTools available inside the development
environment without requiring the user to do much.

History has shown that asking users to install Python packages is prone
to error and has issues interacting with m-c's virtualenv. This command
does things in the least risky way possible. It should be a sufficient
holdover until a dedicated patch uploading command is implemented in
mach. That can be bug 774150.
Attachment #8343544 - Flags: review?(ahalberstadt)
(Assignee)

Updated

5 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment on attachment 8343544 [details] [diff] [review]
mach rbt command to run ReviewBoard Tools

Review of attachment 8343544 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good for an interim first step. Are there any other patch related commands with rbt that might make sense as mach commands (I'm not familiar with the tool). Might be worth creating a new "Patch" category in mach for.
Attachment #8343544 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a4ad5ae7eb7

There's problem room for a "developer productivity" category. Maybe the next command breaks the camel's back.
Blocks: 774150
Flags: in-testsuite-
(Reporter)

Comment 9

5 years ago
I think adding rbt to the development environment is a good first step.

I've got a patch up against RBTools[1] to make it easy for third-parties to register new rbt commands, so perhaps what we can do is (eventually) augment gps's work here to include another package that adds a bunch of Mozilla-specific commands to rbt (for example, mozpost, which would wrap the built-in post command, but could also attach the link to the review request to a Bugzilla bug).

Or we could add such commands to mach. I don't really feel strongly either way.

[1]: https://reviews.reviewboard.org/r/5046/
https://hg.mozilla.org/mozilla-central/rev/6a4ad5ae7eb7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
(Assignee)

Comment 11

5 years ago
Pushed a followup fix to argument parsing. I should have tested this patch more. Change was trivial, so no review needed IMO.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c5a87384e5b
(Since this was morphed to something else entirely.)
Summary: Write a mach command to post patches to Review Board and post links to Bugzilla → Add a mach rbt command to run ReviewBoard Tools
status-firefox28: --- → fixed
Whiteboard: [qa-]
(Assignee)

Updated

4 years ago
Blocks: 1103052

Updated

7 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.