Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,31 @@ If the issue is in Arm Toolchain itself, or if you are not sure, please create a
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?


All contributions to Arm Toolchain are reviewed through GitHub Pull Requests.
If you are asked to review a patch, or would like to understand the review process, please follow the guidance below.

How to Review a Pull Request

When reviewing a PR, you can provide feedback, approve the PR (if you have the required permissions), or request changes
using GitHub’s standard review options:
* Comment – leave feedback without blocking the merge.
* Approve – Approve merging the changes proposed in the PR provided you have the necessary permissions.
* Request changes – If a reviewer selects this option, the identified issues must be fixed before the PR can move forward.
The reviewer who requested changes should re-review the PR after updates are made. The PR should not be merged until the concerns
are resolved and the review is either approved or dismissed. If the original reviewer is unavailable, another authorized reviewer may approve or dismiss a blocking review if needed with proper justification to allow the PR to merge.

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.

For substantive comments, the author should notify the reviewer after making the required changes. The reviewer
should provide explict confirmation or perform a re-review before making the conversation as resolved. This ensures
that all meaningful feedback is properly addressed and prevents premature closure of important discussions.

For full guidance on how to review patches and use these options, please refer to GitHub’s documentation:
see [Reviewing Proposed Changes in a PR](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request)

### Submit a fix

For a small change, please create a Pull Request as described in
Expand Down
Loading