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
> 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.
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
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