#1 will absolutely happen for 1.8. It's in progress now.

One of our students is writing support for specifying a revision and branch a 
change was pushed as, after you close a review request. That should make 1.8.

Christian


On Mar 12, 2013, at 10:30, Matthew Woehlke <mwoehlke.fl...@gmail.com> wrote:

> 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.
> 
> -- 
> Matthew
> 
> -- 
> Want to help the Review Board project? Donate today at 
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~----------~----~----~----~------~----~------~--~---
> To unsubscribe from this group, send email to 
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at 
> http://groups.google.com/group/reviewboard?hl=en
> --- 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 to reviewboard+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
> 
> 

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
--- 
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 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to