Project

General

Profile

Actions

Feature #5595

open

Group code review mail notification

Added by Olivier Renaud about 4 years ago. Updated about 4 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
24.02.2020
Due date:
% Done:

0%

Estimated time:
Sorting:
Commit Number:

Description

My users recently started using pull requests with code peer review. They think it's working great overall, but they are critical about the mail notifications. This is with RhodeCode 4.18.2.

Basically, they complain about the number of mail notifications. Here is what happens from their point of view:

  • The reviewer reads the diffs, and creates 20 inline comments along the way, in the span of 40 minutes
  • 1 hour later, the author replies to most comments in the span of a few minutes
  • 1 hour later, the reviewer replies to some comments in the span of a few minutes

The last two steps are repeated up to 5 comments, like individual threads of conversation.

The problem is that in the span of a few hours, all the recipients of the pull request (author and all reviewers) have received 83 individual mails "user left a note on pull request", that are rarely exploitable by themselves. People who are online receive a mail every two minutes, and people who are offline receive 83 mails at once when they log in. Each mail content is super clean and nicely formatted, but they lack context to be directly actionable (no mentions of the previous comments of the discussion thread). Some users feel like these comments should be grouped in a single mail, corresponding to a "review session" (some code review tools have a concept of review session, where all the comments and notifications are sent in one shot at the end of the session). Other users think they should not even receive these notifications in the first place, since they don't even participate in the specific discussion thread.

I realize it's a very open ended problem, with no perfect solution. My personal opinion is:

  • Notifications should contain more context to be actionable. Receiving notifications with the message "why would it be the case?" or "done" without a reminder of the previous comments is not helpful.
  • Finding a way to group comment notifications could help. This can be by introducing the notion of review session (explicit), or by waiting 30 minutes before sending notifications in batch (implicit).
Actions #1

Updated by Marcin Kuzminski [CTO] about 4 years ago

Hi Olivier,

We "know" that problem with the current code-review system. We discussed this some time ago and decided to go into a direction fo "draft" comments.
Basically what we want to do is allow people to leave draft comments (permanently saved, but visible only to author) Once all draft comments are placed you'll be able to submit them all at once generating a different notification that the current line-by-line.

This would solve partially the problem of too many emails.

The second part is that we'd like to improve here is 1) allow threads in comments (via reply to and indentations, we already went through the UX part of it, now we'll need to implement it) 2) make the emails have more context which is the hardest part. Probably diff+previous replies is something we want eventually to end up with.

Actions #2

Updated by Olivier Renaud about 4 years ago

Thanks for sharing these details. It seems like a very good plan, I can see it working well for my team.

Actions

Also available in: Atom PDF