Open
Bug 1470340
Opened 6 years ago
Updated 4 years ago
make it clear which version of a diff my review comments applied to
Categories
(Conduit :: Phabricator, enhancement, P2)
Conduit
Phabricator
Tracking
(Not tracked)
NEW
People
(Reporter: heycam, Unassigned)
References
Details
(Keywords: conduit-triaged, conduit-upstream)
A suggestion for an improvement to the UI when reviewing. Please let me know if this is already possible and I just couldn't see it. :-)
Sometimes I will be reviewing a diff and while I am reviewing, the author uploads a new version of the diff. When I submit my comments, they should be associated with the old version of the diff, but I can't find anything in the UI that indicates this. Is there a way to make it obvious that my comments were on a specific version of the diff?
Relatedly, even when the author doesn't upload a new version while I am reviewing, as a reviewer I want to know what new changes I need to review when there have been multiple versions. I want to know what the last version of a diff I reviewed was, so I can go to the interdiff view to show only the changes since that version.
Is this possible, or is there a way we can add this? mozreview showed a little slider that also indicated what version of a patch was last reviewed.
(FWIW I find mozreview's use of version numbers starting from 1, 2, 3, ... to be easier to get my head around than some global version number like 4232, 4233, ...)
Comment 1•6 years ago
|
||
This can *sort of* be done manually. If you scroll your browser down to the diff a toolbar will appear along the top with a hamburger menu. You can click the "List Inline Comments" menu item to be taken to a page listing them all[1], along with the diff numbers they apply to. From this you can tell what the last diff you reviewed is.
That being said, maybe upstream would be open to exposing this information in a more friendly way.
[1] https://phabricator.services.mozilla.com/differential/revision/inlines/<revision-id-without-D>/
Keywords: conduit-triaged
Whiteboard: [phabricator-backlog][phabricator-upstream]
Comment 2•6 years ago
|
||
On https://phabricator.services.mozilla.com/D13125?id=46240#378319 one link correctly identifies diff 46233.
But the others do not.
https://phabricator.services.mozilla.com/differential/revision/inlines/13125/ does know, and the greyed out comment is another hint.
Blocks: 1515556
Keywords: conduit-backlog,
conduit-upstream
Whiteboard: [phabricator-backlog][phabricator-upstream]
Keywords: conduit-backlog
Priority: -- → P5
Updated•6 years ago
|
Priority: P5 → P2
Updated•6 years ago
|
No longer blocks: 1515556
See Also: → https://admin.phacility.com/PHI1254
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•