Display a warning when closing a Pull Request tab without submitting review

[olttwa]

Select Topic Area

Product Feedback

Body

There are 2 ways of leaving comments on a pull request: Add single comment or Start a review.

With the latter approach, I have seen multiple users forgetting to hit Submit review after having completed the review.

As a result of this, comments stay in a non-submitted state and both the author and the reviewer are unaware of this.

I propose 3 ways this UX can be improved:

1. When reviewer tries closing the PR tab, pop-up a warning prompting to Submit the review.
2. After a few hours(say 4?) have passed, auto-submit reviews in draft stage.
3. After a few hours have passed, nudge both the author and reviewer via email to publish their review.

[szalapski]

Thank you for raising this as a need; I agree this is problematic for our team.

I do however object: I'm against number 3, in that most people ignore most GitHub notifications today; adding another won't help anything. I'm also against number 2, because I strongly think comments need to be visible immediately. Changing visibility based on time passing is more confusing.

Your number 1 is good but doesn't go far enough.

We just need a mode where comments are immediately visible and are never "Pending".

[lobsterkatie]

I also like idea 1 (and I'd add, the same warning would be nice when you're creating a PR), but I am a hard disagree on comments being shown immediately. I'd be fine having it be a user preference, but not being able to annotate a PR I'm reviewing as I go and then go back and edit those annotations before anyone else sees them... I guess it would just mean I'd type out all of the comments and not click "add comment" on any of them until I'm done, which is fine, I suppose, but kind of annoying.

Bottom line, I need some way to be able to make comments as I go, and not worry about being wrong. If I'm legitimately mistaken by the time I've read the whole PR, fine, the author can correct me, but if I'm only wrong because I haven't finished reading, I'd like not to either look like an idiot or waste anyone's time correcting an idea that I can correct for myself by the time I'm done reviewing.

[szalapski]

Yes, I agree that it should be a user and/or repo preference.

What kind of team situation do you have that won't be forgiving/understanding if they see your comments a little too early? If you send me a comment and five minutes later you change it, very little is lost. Seems to me like the pending state is a solution without a problem.

[lobsterkatie]

All very nice people. This isn't about them. I just don't want to waste their time replying to a comment it'll turn out I don't mean. But also, who likes getting something obvious wrong in public, however low the stakes? Wanting my review to accurately reflect my total understanding of the changes, rather than my partial understanding halfway through, feels like an uncontroversial goal, no? Also, what if I'm not wrong about something, but by the time I'm done reading I've changed my mind about a suggestion?

Anyway, we've strayed pretty far from the actual topic of this suggestion, which is to have GH yell at you whenever you try to leave a page without submitting something you've started to work on. Regardless of when review comments show up, this feels like it would be a helpful feature if applied across the board to general comments, PR descriptions, issue descriptions, review comments, etc.

[szalapski]

I hear you, I just think the rare need to withhold comments is vastly outweighed by the need to facilitate teamwork with timely comments as soon as possible. Sure, some of your comments will be "read too early", but most of the time you will revise them within minutes. What if the default was to "Submit comment" and the rarely-used other option was "save comment for later"?

But even without that nuance, we should just show all comments all the time for maximum teamwork. This is how every other code review tool works, including Azure DevOps Git PRs, SmartBear Collaborator, GitLab, and Bitbucket. GitHub shouldn't be the peculiar, nonintuitive one here. (I'm okay with this being a repo preference or personal preference, of course.)

[MrQubo]

Having behaviour be changeable would be very bad of public repositories. From the user point of view some repositories would behave one way, and some differently. This would actually make the issue even worse, as my comments would be not visible but only on some repositories, not all.

Nowadays GitHub has a separate button "Add single comment" that submits comment immediately.