1296135 - no way to see the commit message for the old revision of a diff

Status: RESOLVED DUPLICATE of bug 1248008

(MozReview Graveyard :: General, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If you look at:
https://reviewboard.mozilla.org/r/66156/diff/5/
it will show you the same commit message as for:
https://reviewboard.mozilla.org/r/66156/diff/7/

But that's not actually the case.  Revision 5 of the diff had a totally different commit message -- one that as far as I can tell is completely unavailable in the MozReview UI.  In other words, the history of what happened is gone.

This is particularly bad when trying to untangle cases where MozReview matched up patches incorrectly (e.g., bug 1295338).

But in general, MozReview is really bad about showing the old state correctly for old versions of the diff (e.g., this bug, bug 1234279).  Bugzilla attachments are much better at this.  This leads me to prefer bugzilla attachments for patches that might require re-review.

Comment 1

[Gregory Szorc [:gps]]

I know this issue has been raised before. So don't be surprised if it is duped.

If you go to the "review" tab/link/page (https://reviewboard.mozilla.org/r/66156/) and hit the "expand all" button, you'll see a Bugzilla like mostly linear history of the review. It clearly shows when the commit took on a new identity.

I agree it would be nice if the diff view contained the proper description if you were looking at a specific revision. This /might/ be a bug introduced by our customizations to show the commits table.

For MozReview maintainers, the old data is available in the "change descriptions" history.

Comment 2

[Mark Côté [:mcote]]

I don't think the old commit message was ever visible in the diff view, but I agree it would be nice.  As gps says, all this data is available in the "Reviews" view, but it could be more convenient.

Comment 3

[Steven MacLeod [:smacleod]]

Review Board is not designed to show updated information in that top box when moving to a previous diff revision. The idea being that the infobox always represents the current state, and you can view the history by expanding the "Change Descriptions" on the reviews page. If we were to show the current status, what should we display when viewing an interdiff?

Fixing this is going to be *a lot* of work. Maybe should we add a warning with instructions if you view a previous diff revision?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Such a warning would make sense.

I'd note, however, that comment 2 in this bug suggests using the reviews view rather than the diff slider to examine history, whereas bug 1249468 comment 1 suggests the opposite.

It's hard to use MozReview to examine history if I have to jump between two views of history and remember which pieces are broken in which view.

Comment 5

[:glob ✱]

this should be fixed as part of bug 1248008.