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.

Sign up for GitHub

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

Adding MIOpen Contributing Guide #932

Merged
merged 3 commits into from
May 24, 2021
Merged

Adding MIOpen Contributing Guide #932

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bef683a
Adding CONTRIBUTING.md
aserio May 14, 2021
ee1863d
Update CONTRIBUTING.md
junliume May 18, 2021
bd7bed1
Apply suggestions from code review
junliume May 24, 2021
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
54 changes: 54 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# MIOpen Contributing Guide
To ensure the quality of the MIOpen code base, the MIOpen team has
established a code review process to inform developers of the steps
that are required to shepherd a change-set into the repository.

junliume marked this conversation as resolved.
Show resolved Hide resolved
## Creating a Pull Request
No changes are allowed to be directly committed to the develop
branch of the MIOpen repository. All authors are required to
develop their change sets on a separate branch and then create
a pull request (PR) to merge their changes into the develop branch.

Once a PR has been created, a developer must choose two reviewers
to review the changes made. The first reviewer should be a
technical expert in the portion of the library that the changes
are being made in. You can find a list of these experts in
[MIOpen Issue #789](https://github.com/ROCmSoftwarePlatform/MIOpen/issues/789)
. The second reviewer should be a peer reviewer. This reviewer
can be any other MIOpen developer.

## Responsibility of Author
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have code style guidelines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@atamazov, do we have a style available? I did find a page on our wiki related to using Clang Format

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we do not have a dedicated coding style doc. Tidy and formatting checks on CI provide the basis for the programming style automatically. I would also recommend reading https://isocpp.org/wiki/faq/coding-standards and https://google.github.io/styleguide/cppguide.html for every MIOpen developer.

The author of a PR is responsible for:
* Writing clear, well documented code
* Meeting expectations of code quality
* Verifying that the changes do not break current functionality
* Writing tests to ensure code coverage
* Report on the impact to performance

## Responsibility of the Reviewer
junliume marked this conversation as resolved.
Show resolved Hide resolved
Each reviewer is responsible for verifying that the changes are
clearly written in keeping with the coding styles of the library,
are documented in a way that future developers will be able to
understand the intent of the added functionality, and will
maintain or improve the overall quality of the codebase.

## Passing CI
The most critical component of the PR process is the CI testing.
All PRs must pass the CI in order to be considered for merger.
Reviewers may choose to defer their review until the CI testing
has passed.

## The Review
During the review, reviewers will look over the changes and make
suggestions or requests for changes.

In order to assist the reviewer in prioritizing their efforts,
authors can take the following actions:

* Set the urgency and value labels
junliume marked this conversation as resolved.
Show resolved Hide resolved
* Set the milestone where the changes need to be delivered
* Describe the testing procedure and post the measured effect of
the change
* Remind reviewers via email if a PR needs attention
* If a PR needs to be reviewed as soon as possible, explain to
the reviewers why a review may need to take priority