I'm a very new RB user and have been tasked with evaluating it to see if it is a tool that we want to roll out for the development team at my workplace. As such, I've spent the last few days experimenting and trying to think of the concerns that other team members may have.
One item I immediately suspected as an annoyance for some team members was the fact that (often lengthy) review entries will always be expanded, particularly when they have already been viewed and there are no new comments. As I dug in, I discovered the new feature from UCOSP 2010 by Kevin Quinn related to Collapsible Reviews (http://www.reviewboard.org/news/2010/12/11/ucosp-2010-project-screencasts/). At first I was convinced that this feature did not actually work or had been removed, because I was not seeing reviews collapse as I anticipated. It wasn't until I checked out the actual code in reviewboard/views.py that it made sense. Here is my interpretation of the code and I'd like to see if someone can confirm this: 1) All reviews since the last update to the review request (or since initial creation if there are no updates) will *always* be *expanded *regardless of whether they have already been viewed. Furthermore, all reviews prior to the last update will be *collapsed *unless #2 applies. 2) Reviews prior to the last update will be *expanded *if a comment/reply has been added since the review request was last viewed. On a subsequent refresh of the request, these reviews will again be *collapsed *unless additional comments have been added. For reference, here is what I believe to be the relevant code from views.py: 442: # Mark as collapsed if the review is older than the latest 443: # change. 444: if latest_timestamp and review.timestamp < latest_timestamp: 445: state = 'collapsed' 446: 447: latest_reply = reply_timestamps.get(review.pk, None) 448: 449: # Mark as expanded if there is a reply newer than last_visited 450: if latest_reply and last_visited and last_visited < latest_reply: 451: state = '' The sticking point for me was the condition on being prior to the last update which triggers the ability for something to be collapsible. I was working with a sample request where I had entered several reviews, but had not updated the request, and was confounded by the fact that none of the old reviews were auto-collapsing. So, if my understanding of this functionality is correct, then I will raise the issue of why does it behave this way? Why make collapsiblity conditioned on an update? Since I'm new to RB, I feel like I must be missing something conceptual about why you wouldn't want to make all viewed reviews (without new comments/replies) auto collapse? I made the following simple replacement to line 444 and it seems to achieve the functionality I am speaking of: if last_visited and review.timestamp < last_visited Would a change like this, perhaps selectable as a user config option, be something that would be considered for inclusion in RB? I'd be happy to open a feature request ticket. Additionally are there any other improvements to collapsible reviews in the pipeline? Kevin Quinn's video mentions cookie based tracking of expanded/collapsed as a item for future work. On a related, but somewhat orthogonal, note, another feature I'd like to see is an improved highlighting of what has changed and/or what is new when viewing a review request. Now, with the current collapsibility logic, I may enter a request to find that a prior review has been expanded which indicates that a new comment has been added. This review may be quite lengthy and span several screens. As best as I can tell, I now have to scroll through this review manually looking at the timestamps of all comments to determine which are indeed new. If these new comments could be highlighted, starred, or somehow otherwise made more obvious then it would make this a much quicker process. It might even be nice to have keyboard shortcuts here to navigate between new comments, similar to what is available on the reviewing page. Again I'd be happy to open a feature request ticket if necessary. Thanks, Griffin Myers -- 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.