1554: Replace "Summary: " with "Review <#> - "

nah****@gmai***** (Google Code) (Is this you? Claim this profile.)
March 27, 2010 
What version are you running?
Review Board 1.5b1

Describe the enhancement and the motivation for it.
In summary area, the current output is "Summary: <summary> [edit]". I think
it would be more useful to put something similar to what Bugzilla has:
"Review <#> - <summary> [edit]". 

Reasoning:
- "Summary" doesn't really bring anything useful.
- In the review page, the review number is currently not listed anywhere
except the URL itself. If one want to write the number somewhere (like in
an email or something), one has to "hunt" for it.
- When using post-review, it is useful to have the review number in the
commit message. With the review number next to the summary, one can just
copy/paste the summary line in his commit message.

For clarification, here is our intended workflow:
- user A commits a fix in the source tree with message "Fix bug XYZ"
- a post-commit hook adds a review in Review Board via post-review
- user B adds a comment to the review
- user A commits a fix with the message "Review A: Fix bug XYZ", copy/paste
from the summary
- the post-commit hook notices the "Review A" message and updates the
existing review with the new diff instead of creating a new one.

Note: this feature was also requested in issue 657
#1 nah****@gmai***** (Google Code) (Is this you? Claim this profile.) 
Replace "Summary :" with "Review <#> - "

Note: I get a "EmptyDiffError at /r/new/" crash when I try to submit the patch to
http://reviews.reviewboard.org
  • +
    review_number.patch 
    --- a/reviewboard/templates/reviews/review_request_box.html
    +++ b/reviewboard/templates/reviews/review_request_box.html
    @@ -6,7 +6,7 @@
     
      <div class="header">
       {% star review_request %}
    -  <div id="summary_wrapper"><label>{% trans "Summary" %}: </label><h1 id="summary" class="editable">{{review_request_details.summary|escape}}</h1></div>
    +  <div id="summary_wrapper"><label>{% trans "Review" %} {{review_request.id}} - </label><h1 id="summary" class="editable">{{review_request_details.summary|escape}}</h1></div>
       <p id="status">
        {% blocktrans  with review_request_details.last_updated|timesince as lastupdated %}Updated {{ lastupdated}} ago {% endblocktrans %}
       </p>
david
#2 david 
"Summary:" does add something, in that it explains to users what goes there if they're 
creating a new review request when uploading a diff on SCMs that don't have changeset 
descriptions (pretty much anything but perforce). We used to have nothing, and a lot of 
people got confused about what it was. Having the ID # there instead doesn't help with 
that problem.
  • +WontFix
#3 nah****@gmai***** (Google Code) (Is this you? Claim this profile.) 
There is another solution to avoid the confusion: have the "summary" text in the
field itself when it's empty, in light gray, and which disappears as soon as the text
box gets the focus. Similar to what a lot of websites have for "search".