Skip to content

Add Reviewer Guidance: Comment/Approve/ Request Changes.#639

Open
simpal01 wants to merge 2 commits into
arm:arm-softwarefrom
simpal01:add-review-section
Open

Add Reviewer Guidance: Comment/Approve/ Request Changes.#639
simpal01 wants to merge 2 commits into
arm:arm-softwarefrom
simpal01:add-review-section

Conversation

@simpal01

@simpal01 simpal01 commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@simpal01 simpal01 requested review from a team as code owners December 8, 2025 17:05
voltur01
voltur01 previously approved these changes Dec 8, 2025
Comment thread CONTRIBUTING.md Outdated

How to Review a Pull Request

When reviewing a PR, you can provide feedback, approve the changes, or request updates using GitHub’s standard review options:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would everyone have the permissions to approve or request changes?

Comment thread CONTRIBUTING.md Outdated

How to Review a Pull Request

When reviewing a PR, you can provide feedback, approve the changes, or request updates using GitHub’s standard review options:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When reviewing a PR, you can provide feedback, approve the changes, or request updates using GitHub’s standard review options:
When reviewing a PR, you can provide feedback, approve the PR, or request changes using GitHub’s standard review options:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for this suggestion

Comment thread CONTRIBUTING.md
Please attach any support files to the issue itself,
see [Attaching files](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/attaching-files).

### Reviewing Patches

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a context for these changes? Otherwise it seems like stating the obvious. Is this for engineers who have not worked with Github before?

@saturn691 saturn691 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Kiran's comments

Comment thread CONTRIBUTING.md Outdated

How to Review a Pull Request

When reviewing a PR, you can provide feedback, approve the changes, or request updates using GitHub’s standard review options:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for this suggestion

Comment thread CONTRIBUTING.md

Resolving review conversations

The author may directly resolve minor comments such as nits, formatting adjustments or other trivial issues.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be confusing as "resolve comment" may mean fix the code, while here this specifically refers to click Resolve button on a comment.
Below it says "resolve the conversation" which is already a bit more specific, but we may want to be very clear.

Also, you may try to rephrase by starting with why, e.g. along the lines of

In order to allow reviewers to remember the context of requested change, do not resolve the conversation, but comment how it was fixed.

then go on to say that minor changes that are accepted as requested can be resolved by the author.

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

Successfully merging this pull request may close these issues.

4 participants