Maybe one solution would be to move the comment box to the top.

That wouldn't really change much, it might even be annoying at times since you typically read through the commit all the way to the bottom, and typically that is the point in time where you want to accept the commit.

I realize I mentioned having to scroll as one of the issue, but the biggest issue is the lack of convenient workflows for accepting many commits.

I don't want to comment on this specifically, just that as a product designer I hesitate when deciding between making things super easy (less quality) or some friction (pause and reset mindset). Friction isn't a bad thing, especially when you need more information or more thorough review. Because Phabricator is a tool for completing quality work, making things too easy seems not as important over ensuring some quality.

How many commits does the average person review in a week? Are there people who essentially just live in Audit? This type of "next" interface could be essentially added anywhere in Phabricator (next Needs Triage task, next Diff, etc).

In T5815#8, @chad wrote:

I don't want to comment on this specifically, just that as a product designer I hesitate when deciding between making things super easy (less quality) or some friction (pause and reset mindset). Friction isn't a bad thing, especially when you need more information or more thorough review. Because Phabricator is a tool for completing quality work, making things too easy seems not as important over ensuring some quality.

I see your point, but there are other concerns as well. In our company our style is to do many small commits, and we just don't have the manpower to do a review for every change. Auditing has become our last resort to ensuring all of our code gets looked at twice before shipping.

Looking at our git log, we have around 15-20 commits a day, with a team of 6 that's around 3 audits a day. The majority of these commits change less than 15 lines of code, and with a team somewhat skeptical to new tools with a sprinkle of RSI on the top, I hope you can understand why I'm trying to cut down the amount of clicks needed for a basic audit. Peer pressure should ensure people spend time on the audits, not intentionally cumbersome interfaces.

Bugzilla has this feature, and we use it during frequent bug scrub meetings for efficiently walking through a certain list of bugs in order to keep them up to date.

I'm confused about how you can not have the manpower for review before commit (Differential) but do have the man power to review after commit (Audit)...? These are theoretically the exact same time commitment, though in theory audit will ultimately be more time consuming since issues are harder to fix after they've been committed, possibly deployed to production, etc but I digress.

The only way I can understand this is if in the audit case the review isn't being done to the same quality as it would be in the differential case. As @chad said, this is supposed to be a tool for quality, thoughtful work, not a tool to process a bunch of crap as quickly as possible.

I'd say ~3 audits per day, assuming you actually review it (e.g. you won't just be hitting "accept" as fast as possible), if all 3 just so happened to be accept folks can suffer through the extra 6 seconds or so total, and otherwise they need to "raise concern" or whatever anyway. These 6 seconds extra across the three audits should be a minute amount of time compared to reading the code, even if each change is a mere 15 lines long.

Long story short, I think any of these "get through this in one click" buttons are a bad product idea, and workflows that would benefit from them should be re-jiggered.

That said, "go to next" in a series of things seems much more reasonable to me. I'm going to morph this ticket to that effect.

Realistically, one guy handling a series of up to 10 commits will not be a rare occurance; if you start auditing a commit series it doesn't really make sense to just quit in the middle.

I still do think reviews involves more commit logistics and terminal work, and it's often awkward to split a longer continuous work into reviewable chunks (for example with the porting I am involved with right now which has going for months). But I would like to add that I of course agree that reviews produce better commits.

Either way, I'm not hoping to spawn a heated discussion here, and I am very glad to hear that the "go to next" feature is of interest.

One last alternative for the accept-shortcut: the comment editor already has <Ctrl-enter> as a shortcut for pressing submit. Would binding something like <Ctrl-shift-enter> to "accept and submit" be unthinkable?

I don't think we'd be interested in that for the upstream. Maintaining a local patch defaulting the action to "Accept Commit" seems like the shortest path for you right now.

Not directly related, but we also had a "Comment and Close" shortcut button in Maniphest previously (similar to GitHub) but eventually removed it because it was a continual source of mistakes and confusion. You can fish around related objects of T4657 / D9221 for discussion. I think adding this to audit (or Differential) is a harder sell than Maniphest was, and it didn't work out there.

Narrowly what some people have told me is is that the flow with the back button(s) to get the the "next" audit is annoying. I think if I've read it right that's what Chad's "work through a queue" description is trying to address.

For me having a way to do "action" then next would be perfect. So I could accept or raise concern and move onto the next audit without having to go back. What I imagine is an extra submit button like [Submit & Next] [Submit].

This would save me a load of time and clicking through 20+ tabs.

This is blocked by T8783 (building Nuance to initial release quality) and T5307 (taking actions with search results).

T8783 is on our natural roadmap but is a large amount of work (at least a solid week of real development).

T5307 isn't huge, but isn't trivial, either.

The result would be a new query result action, Import Items into Nuance... or similar. This would bring them into some kind of internal Nuance queue with application actions.

Definition of application actions probably depends partly on T9132. This is prioritized elsewhere, but also a large amount of work.