Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolving comments #35

Open
CyberMew opened this issue Sep 21, 2020 · 2 comments
Open

Resolving comments #35

CyberMew opened this issue Sep 21, 2020 · 2 comments

Comments

@CyberMew
Copy link

Not sure if this was addressed in the article yet. Who should be in charge of resolving the comments? Should the reviewee resolve it on their own when they think it is fixed, or should the reviewer come back and verify the changes and resolve the thread? Who has the final decision?

@ninachen
Copy link
Collaborator

ninachen commented Oct 7, 2021

(Somehow missed this issue, apologies.)
I think either is fine as a cultural norm as long as everyone is on the same page about it.

If you want to know what Google does, we recommend that authors (reviewees) resolve comments when they've addressed the comment and leave the comment unresolved if still needs action. We do have a section that addresses this, although it is not mirrored to github because most of the content is about specifics of what buttons to click in Google's internal code review tool.

I've copied an abridged version below:


Responding to Comments {#responding}

Unresolved comments are highlighted in the ... UI and await a response. You
are expected to resolve all comments before submitting a CL.

[If you have applied the suggestion, mark the comment as resolved.]

[If you have NOT applied the suggestion], reply to the comment with an
explanation of what you chose to do — and why. See the section on
thinking for yourself. The practice of explaining your decisions will
reduce the number of iterations it takes to review your code and will provide
useful context to reviewers and future code readers.

Prefer leaving comments unresolved when further discussion or action is likely
necessary.


I'm not immeeediately sure whether it's possible to insert a public-friendly version of this "Responding to Comments" section in our docs without munging the internal version.

@ninachen
Copy link
Collaborator

ninachen commented Oct 7, 2021

And of course, even in the abridged form, this assumes that wherever the code review is occuring supports resolving vs. unresolving comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants