Feature Request: Pick which revision or upload to diff against

32 views
Skip to first unread message

Gabriel Morin

unread,
May 28, 2019, 7:18:22 PM5/28/19
to Review Board Community
Hi!

I'm looking into using ReviewBoard with Perforce, and being used to CodeCollab I find that your product holds up surprisingly well in some aspects, in addition to having a slicker UI.

One major useful feature in Collab however is the ability to change what you are diffing against when viewing a file diff, namely any upload that was done to update the review, or any committed revision of the file if it was modified in source control since the start of the review. See attachment for a screenshot where the file wasn't updated externally, so only revision #3 shows. If I had to merge other people's changes, there would be a Version #4, Version #5 and so on, which correspond to the Perforce file revisions.

Swarm added a similar functionality in recent versions: see first screenshot here: https://www.perforce.com/manuals/swarm/Content/Swarm/basics.diffs.html#Viewing_a_diff - it's becoming a pretty standard feature. (Direct link to screenshot: https://www.perforce.com/manuals/swarm/Content/Resources/Images/swarm-panel-diffs.png)

I was wondering if you're planning to add this to ReviewBoard, and whether there are major technical obstacles to implementing that. Honestly if it did have this feature it would become a credible CodeCollab replacement in my view, with the added benefit of being open-source.

Thanks,
Gabriel
SmartBear CodeCollab pick which file to diff against.png

Christian Hammond

unread,
May 28, 2019, 9:42:33 PM5/28/19
to revie...@googlegroups.com
Hi Gabriel,

I *think* the UI feature you're discussing is the same as our diff revision slider. Every time you upload a new diff for review (after making changes based on review feedback or otherwise iterating on the change), we give it a new diff upload revision on that review request. The diff viewer lets you change both the starting diff revision (upstream or any uploaded diff) and the ending diff revision, showing the differences between those.


The same slider is used to change the diff range for file attachments as well. This works for all text-based files, rendered Markdown files, image files, and PDF files (when using Power Pack).

Is that what you're referring to? It looks like it from the screenshots and the linked documentation.

Christian

--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/7dee50f3-51c5-4fe6-8176-14b1e111773b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Christian Hammond
President/CEO of Beanbag
Makers of Review Board

Gabriel Morin

unread,
Jul 5, 2019, 11:21:37 AM7/5/19
to Review Board Community
Hi Christian,

No, I'm specifically referring to this ReviewBoard limitation (documented at https://www.reviewboard.org/docs/manual/3.0/users/reviews/reviewing-diffs/):

Note
Due to the way that the diff viewer works, if a newer diff is based on a newer revision of a file, you may see other changes made to that file between those revisions that has nothing to do with the review itself.
If you’re a developer posting code and you want to sync your source tree, it’s best to try to keep as many revisions of your change based on the same revision of the source tree as possible, in order to minimize the impact of this on your reviewers.

Swarm and CodeCollab in recent editions don't have this limitation anymore, because in addition to doing the same thing as your revision slider, you can change the base revision for the comparison.

For instance in CodeCollab, if I:
- Upload a review when a file is at revision 20
- Sync the file to latest in Perforce (revision 21)
- Update the review

As a reviewer I can hide the changes between revision 20 and 21 from the diff and while still seeing all changes done by the review author, by selecting revision 21 as the base:

2019-07-05 11_00_19-Diff_ LiveConfig.cfg.png


Every feature of the revision slider is still present, for instance I can do this to see the differences between two uploads:

2019-07-05 11_16_15-Diff_ LiveConfig.cfg.png


With reviews lasting sometimes several months and mandatory daily merges, changing the base revision for the diff allows you to hide potentially hundreds of changes that don't come from you from the review - pretty much a killer feature.
It's pretty much the only thing missing from ReviewBoard to make it a serious contender to replace CodeCollab and Swarm for my team - really hope it can be added!

Thank you,
Gabriel
To unsubscribe from this group and stop receiving emails from it, send an email to revie...@googlegroups.com.

Gabriel Morin

unread,
Jul 19, 2019, 9:57:49 AM7/19/19
to Review Board Community
Hi Christian,

I guess this question feel through the cracks, would it be possible to have your take on whether this feature will be added to RB in the future?

Christian Hammond

unread,
Jul 21, 2019, 4:56:46 PM7/21/19
to revie...@googlegroups.com
Hi Gabriel,

I’m sorry, it did fall through the cracks.

I’m still not sure I’m getting it. So right now, in Review Board, say you have this scenario:

1) Post a change for review (diff 1, based on #20)
2) Sync, make some further changes and post again (diff 2, now based on #21)

The revision slider would allow you to view:

1) Diff 1’s changes (against #20)
2) Diff 2’s changes (against #21)
3) Diff 2’s changes against Diff 1 (this is an interdiff)

Interdiffs do filter out upstream changes (new content synced in-between the two diff uploads), but what the documentation is saying is that you may still see some changes show up if they are woven into lines that diff 2 is modifying, or in some cases directly around those lines. This all depends on how the diff algorithm is seeing those changes, and whether our heuristics determine that those changes can be filtered out or are best shown.

I’m having trouble understanding what option #4 for the slider would be, in your example. Diff 2 against #20?

Christian


On Fri, Jul 19, 2019 at 06:57 Gabriel Morin <gabrie...@gmail.com> wrote:
Hi Christian,

I guess this question feel through the cracks, would it be possible to have your take on whether this feature will be added to RB in the future?

--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/2640d340-e49f-4295-8257-0b388c2957b0%40googlegroups.com.
--
Reply all
Reply to author
Forward
0 new messages