100: Allow for reviews based on an existing committed revision in the web UI

daveva*******@gmai***** (Google Code) (Is this you? Claim this profile.)
david
david
Aug. 13, 2013
2349
What's the URL of the page containing the problem?
http://demo.review-board.org/r/new/

1. I'd like to be able to request a review of already checked-in code by
simply selecting a "{revision_number} - {revision_comment.substring}" from
a drop-down box based on the selected repository.
david
#1 david
  • -Type-Defect
    +Type-Enhancement
  • +Allow for reviews based on an existing committed revision
david
#2 david
  • +Component-Reviews
#3 mge****@gmai***** (Google Code) (Is this you? Claim this profile.)
I second the wish for reviewing already committed code.  It would be especially
useful for reviewing code on other branches prior to merging them.

I'm not sure about a drop-down box.  The Zope 3 subversion repository already has
over 70 thousand revisions, that's a bit much for a drop-down.
#4 John.L.H*********@gmai***** (Google Code) (Is this you? Claim this profile.)
As an alternative to selecting revisions from a list would you be open to using two
integer fields to choose a range of revisions for a given repository?

For background: we have one development and one production server at work so we
generally check-in changes on dev and review them there before making them live.  We
will sometimes consider several minor revisions as a common change set so we would
prefer to diff a range as opposed to stepping through each minor revision.
#5 dab****@gmai***** (Google Code) (Is this you? Claim this profile.)
I agree with John; this would be extremely useful.  We have a similar environment
here and it would be great to be able to auto-generate diffs based on committed
revisions instead of producing the diffs manually.
#6 philippe********@gmai***** (Google Code) (Is this you? Claim this profile.)
Hey,

I patched post-review to add a new option: --revision-range
No UI integration, but post-review seems quite easy to use ;)
http://reviews.review-board.org/r/162/
#7 jtr****@gmai***** (Google Code) (Is this you? Claim this profile.)
Any chance of this getting a bump in priority?  Right now we're evaluating several
different code review tools and this feature would make RB my fave.
#8 matt.******@gmai***** (Google Code) (Is this you? Claim this profile.)
I agree that this would be extremely useful, especially if it allowed specify things as:

branch1 revision#
branch2 revision#
david
#9 david
Does the post-review implementation work for people?
#10 phil.******@gmai***** (Google Code) (Is this you? Claim this profile.)
Other non-svn clients need support for this. Also, this should be doable from the web UI.
chipx86
#11 chipx86
We're trying to keep from adding new features until 1.0 goes out. I have some ideas
for this in the web UI for 2.0.
  • +Milestone-Release2.0
#12 eric****@gmai***** (Google Code) (Is this you? Claim this profile.)
I worked on the original changes to post-review to support this for Perforce. 
However, it doesn't support ranges for Perforce.

I'm not sure if I should open a separate issue for this, but I would like to have the
ability to post a review for an entire, existing file (range #0 to #head in perforce
parlance).  For example, if there is a file in the repository and maybe it's been
around for a few years and has had many changes to it and has gotten a little crusty,
I would really like to be able to post the entire file up for review (independent of
a changeset or diff).  It should show up as-if it were an "added" file (green, single
column display).  Perhaps reviewboard could even leverage the interdiff magic and
show all committed revisions of that file.
chipx86
#13 chipx86
  • -Priority-Medium
    -Milestone-Release2.0
    +Priority-High
    +Milestone-Release1.5
  • -Allow for reviews based on an existing committed revision
    +Allow for reviews based on an existing committed revision in the web UI
david
#14 david
  • +Confirmed
#15 nithi******@gmai***** (Google Code) (Is this you? Claim this profile.)
We have a similar requirement. We would like to be able to create a review of checked
in files but giving a list of files, and a date range or label range in perforce.

Looking forward to this feature.
#16 mar***@maine.****** (Google Code) (Is this you? Claim this profile.)
I have a request from our team for something similar to this.  We'd like to be able
to have reviewers self-select committed revisions for review and feedback. 
Essentially, a lead reviewer would browse a range of svn checkins (the previous
day's, for example) from a repository or a selected subtree of a repository, be able
to view the diff and review the code (or publish the diff for wider review) on the spot.
#17 cybr****@gmai***** (Google Code) (Is this you? Claim this profile.)
I would like to also comment that this feature would be VERY useful for us.

The post-review process is cumbersome and requires all developers to have python
installed on their machines and another script to maintain/update. 

Instead of drop down boxes the user should simply type-in the revision number. This
is both simple to implement and more useful if there are too many revisions.
#18 Stefan.B********@gmx**** (Google Code) (Is this you? Claim this profile.)
+1 vote for this enhancement.

A simple edit field to type in rev numbers should be suffficient.
Drop down boxes would be nice to have but should really be useable with big
repositories storing thousands of revisions.
chipx86
#19 chipx86
  • +Milestone-Release1.6
#20 eric.t*******@gmai***** (Google Code) (Is this you? Claim this profile.)
I think this is huge for adoption. Making it easier to generate review requests.

Check out Kiln from Fog Creek ( same guys that make FogBugz ). It is kind of annoying 
how it implement Mercurial integration, but the review and integration with FogBugz 
is great.

It's post commit...but generally you commit to a develop branch, review, and then 
push/merge to trunk...and all from the web which is nice. Just seeing a history log 
from your repository is nice.
#21 Jan.Ko*******@gmai***** (Google Code) (Is this you? Claim this profile.)
This will be great :) especially for ClearCase :]
chipx86
#22 chipx86
Moving some bugs/requests to 1.7. 1.6 is intended to be a short release, and needs to be limited in scope.
  • +Milestone-Release1.7
david
#24 david
Ahahahaha comment #22.
  • -Milestone-Release1.7
    +Milestone-Release1.8
#25 drma*****@gmai***** (Google Code) (Is this you? Claim this profile.)
This bug is years old now, has it been included in any existing release?
david
#26 david
  • -Confirmed
    +Started
  • +david
david
#27 david
I've just pushed the critical change to master (20b6498). This functionality will ship in 1.8, at least for GitHub and SVN repositories. Whoo!
  • -Started
    +Fixed