#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 <[email protected]> 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 > [email protected] > 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 [email protected]. > 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 [email protected] 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 [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
