Detailed comments below, but summarizing my personal short term priorities:

1. Custom field support.
2. Support DVCS revision in the changenum field.
3. Add a field for local branch name.

Of course, (1) means that (3) (and (2), sort-of) can be done with an extension... which is one reason (1) is first, but also because we have local need for custom fields that are probably not useful to a wide enough audience to justify being built-in. This lack is the biggest pain point for us right now.

I think pushing some things (especially patch set support) to 2.x sounds reasonable, but please, *please* at least have custom field support in 1.8 :-).

(Besides, a number of the other features can be implemented to at least some extent with better extension support. This would be a good path for a wider developer base to work on them, and/or to 'try them out' conceptually before bringing them into RB core... if that is even valuable in the long term. It may end up that some features work so well as extensions that there is no strong reason they can't just stay extensions.)

On 2013-03-12 08:52, Stephen Gallagher wrote:
[paraphrasing Christian:]
I absolutely want to introduce patchset support. My thought is that
post-review (well, rbt post, the successor coming in RBTools 0.5) would
allow you to optionally post a series of commits as one patchset,
instead of squashing them.

Review Board would be able to differentiate patchsets vs. standard
diffs. When a review request doesn't contain any patchsets, it'd be just
like it is today. However, when a patchset is involved, we change things
a little bit.

The revision selector is still there, but a patchset now counts as a
revision. The patchset will show each and every commit, along with their
metadata (descriptions and such). They could be viewed squashed (same
view as today), or per-commit.

So... while I understand that this is technically more feasible, I'd be concerned with how it will handle e.g. rebases. Also, I think it will be much more logical for reviewers to keep commits within a request separate from revisions of the request, rather than mixing these concepts.

Basically, unless you're talking about a project with a draconian 'no rewriting history' policy, request revisions and commits are different (and incompatible) concepts. Trying to shoehorn the one into the other is likely to result in cases where the user experience is less than it would be if they were treated as being different.

We'd have some intelligence to try to see if a patchset was just
amended when updated next, or if part of it was replaced, or what.
We'd still have a new patchset entry internally, but we'd take
advantage of some of our diff data sharing to see what the common
parts are. We can then indicate to the reviewer that commits were
added, or changed, or where. That'd make it easier to see what
commits actually need to be reviewed.

I still like my suggestion better; continue to upload a single patch (which would create a single request revision), but accept patches that are really 'git am' chains. Then detect these server-side and add a /second layer/ with the option to inspect individual patches.

This should still be able to take advantage of diff caching and similar. Detection of added/removed/reordered patches can then be done using the usual diff mechanisms on the complete patch files (some heuristics will be needed to interpret the results, but the actual comparison can use existing algorithms).

Along with this, there'd be some other improvements. We'd have a field
indicating where the development branch/repository is (so that people
can pull from you, if it makes sense in their setup), and the
branch/revision where the commit was made (as part of the submitted
form, and also available to set via new RBTools commands and options).

I expect I will rehash the above paragraph, but... IMO, minimum requirements for decent DVCS support are two branch fields: one the merge target branch (i.e. 'master' most of the time), and the name of the head as pushed to the remote. Also a third field with the actual SHA of the request (I would prefer to have this in the existing changenum field; it seems to equate with what that field is intended to be).

I also wouldn't be worried about multiple repository support initially; even some public open source projects use single repositories with either branch-level commit access or just plain trusting people... and this is probably the more common case (in fact, I would expect not having a single canonical repository for code sharing to be rare) in corporate settings.

On a related note, it would be a fantastic feature if we could also tie
this into github pull requests. It looks like the github API[1] allows a
client to register for notification of pull requests. It would be
excellent to be able to automatically-generate a ReviewBoard review for
the requested branch.

I wouldn't even try to do this in RB proper in the short term (if ever); it's too obviously excellent fodder for an extension or external tool... which means it can be developed in parallel without affecting RB's release schedule.

The only possible reason I can think of why it would ever need to be fully integrated is if it is not otherwise possible to provide reasonable security (i.e. if it would not be possible to secure access to the well-permissioned user that the tool would need to have to talk to RB).

Similarly, it would be pretty excellent if we could tie in a button to
push the Merge Button™, though that would necessitate adding a
permission feature on ReviewBoard to set who was allowed to perform
merges. We'd also probably want to implement a policy such as "The
review must have at least N 'Ship It!' values before it can be merged."

For the record: this is one of the main reasons people give why RB is not an acceptable alternative to gerrit.

The "at least N Ship Its!" is something that a lot of companies want,
but not all want the same logic. Some want just a total, some want from
certain groups, some from certain people, etc. I think we'd need
something more general that could have an extension for defining such
rules. I think it's separate work from the DVCS work.

This sounds like a good excuse to develop the extension system to allow an extension to implement the logic. Probably this just needs an internal field if a request can be merged that can be set by the extension.


Want to help the Review Board project? Donate today at
Happy user? Let us know at
To unsubscribe from this group, send email to
For more options, visit this group at
--- You received this message because you are subscribed to the Google Groups "reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
For more options, visit

Reply via email to