search

ReviewNB / support

Issues and feature requests for ReviewNB
https://reviewnb.com
58 stars 8 forks source link

Notifications for comment replies #58

Closed neilpeterman closed 4 years ago

neilpeterman commented 4 years ago

As a reviewer, I'd like to be pinged if someone responds to my comment so I can interact without needing to periodically check the PR page.

If someone replies to a comment in ReviewNB, right now these are not leading to Github notifications. I don't see a way to configure Github notifications to fix that, but perhaps there is a way that I missed.

Right now it looks like the comment interface with Github makes a Github comment for each ReviewNB comment, but comment replies are recorded as edits to these original comments.

amit1rrr commented 4 years ago

@neilpeterman Thank you for starting this discussion.

comment replies are recorded as edits to these original comments.

We actually have 2 types of review comments internally,

  1. Regular review comments
  2. Review comments as issue comments (RAIComment)

Replies to regular review comments are posted as normal GitHub replies & the author gets notification for every reply (just tested it again). This is the most common type of review comment.

When the notebook diff is large (>1mb or >20k lines) then GitHub doesn't support posting comments on such large diffs. We built RAIComment as a workaround for large diffs. More information here. These are posted as issue comments on the PR & each reply to RAIComment = editing the original issue comment to add a reply.

Long story short, you don't get reply notifications for RAIComment which are used less often (only when the notebook diff is large). But it's still a limitation & it's annoying to check for replies instead of being notified.

I am going to try out a few simple solutions to trigger GitHub notification. e.g. automatically appending cc: @neilpeterman in the reply to comments made by neilpeterman. Will keep you posted.

amit1rrr commented 4 years ago

tl;dr You should now get email notification for every reply to a conversation you're involved in (even if you are not involved in the conversation you can subscribe to the PR and get notified when new comments/replies are posted)

Feel free to reopen the issue if you see any problems.

Longer version

I am going to try out a few simple solutions to trigger GitHub notification. e.g. automatically appending cc: @neilpeterman in the reply to comments made by neilpeterman

This didn't work. Apparently GitHub doesn't notify on comment edits (and rightly so as most edits are typo and other tiny fixes). Even if you @ someone in the edit they would get a notification the first time but not for subsequent edits. So it wouldn't have worked for us since 2nd reply onwards wouldn't have been notified.

To solve this, we are posting an actual issue comment like below (in additional to the regular edit) to trigger a notification. These comment have a link to PR page on ReviewNB to quickly see the entire conversation. See screenshot below to know how a reply looks on GitHub and ReviewNB,

Screenshot 2020-04-21 at 4 36 28 PM

Screenshot 2020-04-21 at 4 35 41 PM

neilpeterman commented 4 years ago

Following up after seeing the fix in action in a few PRs, works great!