Having Reviewboard be capable of checking in code.
Randy Robertson
the 'Presubmission Build System.' The basic idea is that you drop a patch
(which may have some embedded metadata) into a specific place on a presumably
shared NFS mount, and the system will take that patch, do various builds
and testing, and ultimately reject or submit the change on your behalf to
the underlying source control system.
Many people have expressed that it would be nice to trigger this submission
directly from within Reviewboard.
I can imagine you would have a link 'Submit change' perhaps next to the
|Download Diff|Review|View Diff| options on the summary page for a given change.
This would probably be implemented as a site-specific option to you can enable.
It would need to take as a configurable parameter the patch-directory base and
(at least for my specific use case) would need to dump files inside that
directory in folders with names which match the appropriate SCM user, and branch.
e.g. Reviewboard would just spit out a generated patch file and put it in
/mnt/public/pbs/branch-name/username/959592.patch
Ideally, when you click the 'Submit change' link, you could also be presented
with some checkboxes/fields whose key/values will get embedded into JSON
which is prepended to the contents of the patch. Which fields/checkboxes
would be something data driven and also from a configuration file or script which might
take into account which files/branches are involved.
For example, here is sample change file:
----- PBS Metadata -----
{"dontcommit": true}
----- PBS Metadata -----
From: rmro...@vmware.com
Here is an example change.
This doesn't really do anything
QA Notes:
Testing Done:
Documentation Notes:
Bug Number:
Reviewed by: nobody
Approved by:
Mailto:
---
==== //depot/bogus/branch-name/README#47 ==M== /trees/rmrobert-branch-name/bogus/README ====
*** /tmp/p5diffhtml.tmp1.23406 Fri Mar 5 16:08:05 2010
--- /trees/rmrobert-branch-name/bogus/README Fri Mar 5 16:07:04 2010
***************
*** 1,4 ****
--- 1,5 ----
This is the Bogus source tree.
+ Trivial change to a README file.
The main directories and files are
Even in the absence of something like our PBS, it might be generally useful to extend Reviewboard to allow
it to do submits directly to the underlying SCM (which should be pretty easy, but is a bit more involved
than with PBS since you would need to manage a client/tree for something like Perforce/Git/SVN, etc.
I wanted to hear some feedback from the community about whether you are amenable to adding such
functionality to Reviewboard (or would accept a patch from me that does this).
PBS itself is not currently open sourced, but may be in the future, although since the
interaction with it is really just a matter of spitting out a patch file in the right,
there shouldn't be any licencing issues.
Thanks
-Randy Robertson
Chris Trimble
Randy Robertson
> What about merges / conflicts? It would kinda stink to have to implement a
> bunch of code to deal with that in a web UI.
> I guess if a review was always submitted against tip that wouldn't be an issue,
> but oftentimes large reviews can take a while to get through (in my
> experience).
The expectation is that each patch should apply in isolation to the top of
the tree which has been marked as 'stable.'
The implementation of PBS has to handle lots of crazy merge and conflict scenarios and
will do the right thing (including bounce a later change if it conflicts against another in flight)
But Reviewboard can be blissfully ignorant of all of that. It's job is just to dump the patch in the right place.
PBS will generate emails about every step of the process and has its own complicated UI
to visualize the steps and merging that various changes go through.
Christian Hammond
Given how specific this really is to VMware's architecture, we can't put it into the code product. It will be possible in the future, though, if you guys wanted to build it.
Right now we're working on getting the 1.5 release out the door. After that, we're going to spend a lot more time getting our extensions support in, which would allow people to write extensions to Review Board that, amongst other things, can add actions to the review request page, access the database, etc.
This is a little ways off, of course. We have to finish building it, testing it, getting other features we have planned into the release, and performing the release. So, we're looking at a year or more. But I don't see how we'd get the functionality you want short-term.
There would be a lot of issues with having Review Board itself commit changes, as it would need the proper credentials for each user to even be able to commit, and this may change depending on the SCM. Dropping a patch, as you suggested, would be fine, so long as there's another system that is handling the commits, but that would only be useful inside VMware today. Having PBS open sourced would be great and that would make it easier to add this, but if it's a feature we claim to support, we'd still need it to work with various other SCMs and provide full support for it.
Christian
--
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com
Paul
> Even in the absence of something like our PBS, it might be generally useful to extend Reviewboard to allow
> it to do submits directly to the underlying SCM (which should be pretty easy, but is a bit more involved
> than with PBS since you would need to manage a client/tree for something like Perforce/Git/SVN, etc.
Funny, a few weeks ago I implemented the following patch which has
been working well in our environment:
http://reviews.reviewboard.org/r/1479/
It's to rbtools rather than to the web GUI, but that's a far easier
place to put it (as the call out to the SCM tool can pop up whatever
messages and other things are necessary direct to the user rather than
being proxied by the web UI).
Does this meet your needs?
Do you have other feedback?
I only implemented the full set of changes for subversion because I'm
doing it for $WORK and that's what we use (and I haven't had personal
time to implement non-work related functionality).
Paul
Daniel Laird
running at $WORK and first impression have been very good.
Adding this functionality means descriptions etc get written once and
this is a good thing.
I may try these fixes before they become part of the official RBtools
release.
I like the idea of having a submit this review to the codebase but
fully appreciate the 'what if it does not apply cleanly' problem.
So careful thought would be required - that said the gains would be
very useful.
Dan