Page MenuHomePhabricator

Navigating the repository tree while auditing
Closed, DuplicatePublic

Description

It would be incredibly useful for audits if one could move to the next and previous commit from diffusion.

Nowadays when performing an audit, you are watching the code from diffusion, and to move to the previous / next commit you need to go back, search for the commit you were in, and choose the previous / next one. This is incredibly difficult for such a common and simple task. Especially in a post-commit review cycle, where a single push may contain several commits that need auditing.

Moreover, if there where shortcuts for this, it would be even better!

Obviously moving to the previous / next commit may have more than one possibility (merges and branches), but the options could be easily listed and a sane default used (for instance, keeping in the same branch when possible).

Event Timeline

The parents of the commit should be linked at the top already. Are you not seeing them?

It's relatively expensive to determine the "next" commit(s) -- I think we have to enumerate every branch which contains the current commit and then log from the current commit to the branch head for each one. Maybe there's some clever way to do this, though. (This is easy in Subversion, at least.)

Or do you mean that you want "go to next commit requiring audit by me" (i.e., next thing I need to take action on), not "go to previous/next commit in history"? When I need to look at several things I just open them all in new tabs and then use next tab / prev tab for next/prev. Do you not use tabbed browsing, or is this not a good solution for other reasons?

i was meaning next / prev commit in the revision tree, so you are right, I totally missed the link to parent up there, though having the navigation at the bottom is certainly a UX win, since at the bottom is where you end up after performing an audit.

As for the next commit, that info is already being displayed in the commit history where it's properly graphed, though it may be tricky to transverse the tree to reach an arbitrary commit. Non the less, the way I see it, if you are doing audits, you are checking the commits on a specific branch, so the next commit would generally be the next in line in the same specific branch. This "simplified version" of the next commit should be substantially easier to determine and still incredibly useful for the given use case. When reaching a point where there is no "next", this situation could be easily stated and let the user go back to the history to look for anything else.

Just a clarification, When I say "that's where you end up after performing an audit" I mean assuming there is nothing wrong in the commit. Otherwise you end up at the top after submitting your comments, in which case the current "parent" link works great.

I think your use case (of wanting to audit every commit sequentially) is a bit unusual -- I believe most people audit only some commits (based on rules), and arrive at the audit list either from email or from audit requests on the home screen. Although I don't have hard data to back up this assumption, it's definitely the way we and Facebook use audits and consistent with most of the requests we've received.

In these workflows, we can't determine "next" (at least, not uniquely or cheaply) because there's no branch context: the user arrives at the commit in isolation. And the user's expectation of "next" and "prev", were they to exist, is almost certainly "next / previous audit request", not "next / previous commit". The easiest way to address this might be to implement that, and then you could just write a Herald rule to trigger an audit request for yourself for every commit.

This has come up before (T878) but any implementation should probably span all of the tools and be a generalized "next thing that needs your attention". We're closer to being able to implement this now that Notifications have partially landed, but the concepts are a little different.

Firstly, thanks for your feedback.

One important comment, is that i'm not necessarily checking every commit, just a couple of consecutive commits, such as those that may be pushed together in a post-push audit work cycle.

Anyway, what you propose is a viable option, though it requires configuring Herald rules appropriately.

chad changed the visibility from "All Users" to "Public (No Login Required)".Jul 3 2015, 3:57 AM

I'm going to merge this into T5815, which is a more general discussion of action queues.