At VMware, many people are starting to use a service I've written called 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.
--- ==== //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.
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).
On Fri, Mar 5, 2010 at 4:29 PM, Randy Robertson <rmrob...@vmware.com> wrote: > At VMware, many people are starting to use a service I've written called > 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.
> --- > ==== //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.
On Fri, Mar 05, 2010 at 05:01:27PM -0800, Chris Trimble wrote: > 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.
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.
On Fri, Mar 5, 2010 at 5:19 PM, Randy Robertson <rmrob...@vmware.com> wrote: > On Fri, Mar 05, 2010 at 05:01:27PM -0800, Chris Trimble wrote: > > 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.
On Mar 5, 5:29 pm, Randy Robertson <rmrob...@vmware.com> wrote:
> 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.
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).
I really like these additions (to RBTools). I have got reviewboard 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
On Mar 19, 11:29 pm, Paul <psarm...@gmail.com> wrote:
> On Mar 5, 5:29 pm, Randy Robertson <rmrob...@vmware.com> wrote:
> > 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.
> 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).
