From d37ea367d0b36aec797242e6bb6671fad89ceece Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Mon, 12 Aug 2024 12:08:06 +0200 Subject: [PATCH 1/9] Update CONTRIBUTING.md --- CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bb3fa6d8..4c77fb77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,6 +10,8 @@ We welcome all contributions, suggestions, and feedback, so please do not hesita - [3. Submit Pull Requests](#3-submit-pull-requests) - [How to Create a PR](#how-to-create-a-pr) - [Developer Certificate of Origin (DCO) Sign off](#developer-certificate-of-origin-dco-sign-off) + - [Code Review of the Pull Request](#code-review-of-the-pull-request) + - [Possible problems and quirks](#possible-problems-and-quirks) ## Engage with us @@ -135,3 +137,42 @@ This can easily be done with the `-s` command line option to append this automat ```sh git commit -s -m 'This is my commit message' ``` +#### Code Review of the Pull Request + +In Nephio Project uses [Prow](https://docs.prow.k8s.io/) to among many other tasks test and manage the Pull Requests. Prow also allows to interact with PR trough ['slash' commands](https://prow.nephio.io/command-help) + +##### The Code Review Process +The **author** submits a PR +- Step 0: [Prow bot](https://github.com/apps/nephio-prow) assigns reviewers and approvers for the PR based on [OWNERS files](https://www.kubernetes.dev/docs/guide/owners/) please note that single repository can have multiple OWNERS files, bot will choose reviewers and approvers from file nearest to changed code +- Step 1: Prow tests the PR +If the author is not (yet) a member of Nephio GitHub organization Prow will add label 'needs-ok-to-test' which will prevent Prow from running tests on it. This is a security measure as many tests allow to execute arbitrary code, create objects in infrastructure and so on. To allow the tests to run any member of organization can use command +/ok-to-test in comment, then tests will run. Status of their execution will be visible on PR page itself or if one wants to see logs, previous runs etc those are available on [Prow's Dashboard](https://prow.nephio.io/) +Definions of those tests are either in local [inrepoconfig](https://docs.prow.k8s.io/docs/inrepoconfig/) file .prow.yaml or in [central Prow configuration](https://github.com/nephio-project/test-infra/tree/main/prow/config) +- Step 2: Review of the PR +Reviewers check the code quality, correctness, software engineering best practices, coding style and so on. +Anyone in the organization can act as a reviewer with the exception of the individual who opened the PR +If PR content looks good to them, a reviewer types /lgtm (**looks** **good** **to** **me**) in the PR comment or review; Prow bot will then apply 'lgtm' label to the PR, this can be canceled by removing label or using '/lgtm cancel' command +- Step 3: Approval of the PR +Only people listed in the relevant OWNERS file in section 'approvers', either directly or through an alias, can act as approvers, including the individual who opened the PR. +Approvers check for acceptance criteria, compatibility with other features, forwards/backwards compatibility, API definitions and so on. +If the code changes gets their approval, an approver types /approve in a PR comment or review; this as well can be canceled with '/approve cancel' command +Prow bot applies an approved label +- Step 4: Automation merges the PR +If all of the following conditions are met: + - All required labels are present (lgtm, approved) + - No blocking labels are present (do-not-merge/hold, needs-rebase) + - There are no presubmit prow jobs failing + - Then the PR will automatically be merged + +##### Possible problems and quirks + +- Approval and lgtm process + - Technically anyone who is a member of the Nephio GitHub organization can /lgtm a PR which can be both good (reviews from non-members are encouraged as a way of demonstrating experience and intent to become a member or reviewer) and bad (/lgtm’s from members may be a sign that our OWNERS files are too small, or that the existing reviewers are too busy). Note to approvers (who can do both approve and lgtm) - if possible please leave lgtm to other in the spirit of having at least two sets of eyes on every change. + - Reviewers, and approvers are unresponsive +Please do not rely on GitHub notifications sent to approvers/reviewers or mentions like “pinging @the_rewiever for approval” - those are really hard to filter and reviewers/approvers are very busy people. If your PR doesn't get enough attention consider asking for the review in respective Slack channels. +- Prow things + - If the presubmit job fails there's a message posted by Prow bot that explains steps how to re-run it, for example with command /test my_awesome_presubmit_test + - Sometimes it takes a while to merge the PR after it was approved, PR queue can be observed on [Prow's Tide status dashboard](https://prow.nephio.io/tide) + - If the PR gets final step like approve or lgtm done after 12 hours pass since presubmit tests were run they are getting re-triggered +- GitHub things + - EasyCLA sometimes non-properly check the status, it can be re-triggered with /easycla command or it often sorts out when you close and re-open the PR From bd1e3e44e066ce93d940405b91b1630e3a31dae3 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Wed, 14 Aug 2024 19:01:52 +0200 Subject: [PATCH 2/9] Create COMMUNITY.md --- COMMUNITY.md | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 COMMUNITY.md diff --git a/COMMUNITY.md b/COMMUNITY.md new file mode 100644 index 00000000..759ea84d --- /dev/null +++ b/COMMUNITY.md @@ -0,0 +1,107 @@ +# Nephio Project Community membership + +This document describes the responsibilities and contributor roles in Nephio Project. + +| Role | Responsibilities | Requirements | Defined by | +| -----| ---------------- | ------------ | -------| +| New contributor | - | Submits the Pull Request on GitHub | Anyone is welcome to participate | +| Member | Active contributor | Sponsored by one reviewer and has several contributions to the project | Nephio GitHub org member| +| Reviewer | Review contributions from other members | History of review and authorship in the project | [OWNERS] file reviewer entry | +| Approver | Contributions acceptance approval| Highly experienced active reviewer and contributor to the project | [OWNERS] file approver entry| + +## New contributor + +Anyone is welcome to contribute to Nephio project. Existing members should make a point of being helpful +for new contributors in any way possible (PR workflow, finding relevant documentation, guiding with coding style etc.) +Remember we were all '[new contributors]' once. + +## Member + +Members are active contributors in the community. Members might have PRs and GitHub issues assigned to them and are added to GitHub team(s). +Prow's presubmit tests are automatically run for their PRs as well as GitHub Actions (where applicable). +Members are welcome to remain continously active contributors to the community. + +### Requirements + +- Ensure affiliation is up to date in [openprofile.dev]. +- Have made **several contributions** to the project or community, enough to + demonstrate an **ongoing and possible long-term commitment** to the project like: + - Authoring or reviewing PRs on GitHub, with at least one **merged** PR. + - Filing or commenting on issues on GitHub + - Contributing to SIG, subproject, or community discussions (meetings, + Slack, mailing list) +- Have read the [contributor guide] +- Sponsored by one reviewer +- **Open an issue [Membership request] against the 'nephio' repo** + - Ensure your sponsor is @mentioned on the issue + - Provide information described above that is representative of your work on the project +- Have your sponsoring reviewer reply confirmation of sponsorship +- Once your sponsors have responded, your request will be reviewed by the [GitHub Admin team]. + +### Responsibilities and privileges + +- Responsive to issues and PRs assigned to them +- Responsive to mentions of SIG teams they are members of +- Active owner of code they have contributed + - Code is well tested + - Tests consistently pass + - Addresses bugs or issues discovered after code is accepted +- Members can do `/lgtm` on all PRs. +- They can be assigned to issues and PRs, and people can ask members for reviews with a `/cc @username`. +- Tests will be run against their PRs automatically. No `/ok-to-test` needed or approving GitHub Actions run. +- Members can do `/ok-to-test` for PRs that have a `needs-ok-to-test` label, and close PRs. + +## Reviewer + +Reviewers check code for quality and correctness. They are knowledgeable about both the project (or parts of it) +and software engineering principles. + +### Requirements + +The following apply to the part of codebase for which one would be a reviewer in +an [OWNERS] file. + +- Member for at least 2 months +- Primary reviewer for at least 5 PRs to the codebase +- Knowledgeable about the project and codebase +- Sponsored by an approver + - With no objections from other approvers + - Done through PR to update the OWNERS file: either self-nominate or any other member of the Nephio Project + +### Responsibilities and privileges + +- Tests are automatically run for their PRs +- Responsible for project quality control via code reviews +- Expected to be responsive to review requests +- Assigned PRs to review related to subproject of expertise +- Assigned test bugs related to subproject of expertise +- Granted "read access" to Nephio Project repo(s) + +## Approver + +Code approvers are able to both review and approve code contributions (by setting labels `approved` and `lgtm`). +While code review is focused on code quality and correctness, approval is focused on broad acceptance of a contribution including: backward and/or forward +compatibility, adhering to API, performance and correctness issues, interactions with other parts of project and so on. + +### Requirements + +- Reviewer of the codebase for at least 2 months +- Primary reviewer for at least 10 PRs to the codebase +- Nominated by a project owner or other approver + - With no objections from other project owners or approvers + - Done through PR to update the OWNERS file + +### Responsibilities and privileges + +- Demonstrate sound technical judgement +- Responsible for project quality control via code reviews + - Focus on holistic acceptance of contribution such as dependencies with other features, backwards and/or forwards + compatibility, API and flag definitions, etc +- Expected to be responsive to review requests +- Mentor contributors and reviewers +- May approve code contributions for acceptance + + +[new contributors]: /CONTRIBUTING.md +[OWNERS]: https://www.kubernetes.dev/docs/guide/owners/ +[openprofile.dev]: https://openprofile.dev/edit/profile From b970c8063f11e371612db4f17cf244a6ee8b0d2a Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Wed, 14 Aug 2024 19:04:30 +0200 Subject: [PATCH 3/9] typo fix in COMMUNITY.md --- COMMUNITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COMMUNITY.md b/COMMUNITY.md index 759ea84d..c3210546 100644 --- a/COMMUNITY.md +++ b/COMMUNITY.md @@ -19,7 +19,7 @@ Remember we were all '[new contributors]' once. Members are active contributors in the community. Members might have PRs and GitHub issues assigned to them and are added to GitHub team(s). Prow's presubmit tests are automatically run for their PRs as well as GitHub Actions (where applicable). -Members are welcome to remain continously active contributors to the community. +Members are welcome to remain continuously active contributors to the community. ### Requirements From e84bdc39377ac321a2efcb4b4d8ee3be7760b021 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Wed, 14 Aug 2024 22:27:53 +0200 Subject: [PATCH 4/9] Update CONTRIBUTING.md Co-authored-by: Rahul Jadhav --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4c77fb77..a5be0d8f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -175,4 +175,4 @@ Please do not rely on GitHub notifications sent to approvers/reviewers or mentio - Sometimes it takes a while to merge the PR after it was approved, PR queue can be observed on [Prow's Tide status dashboard](https://prow.nephio.io/tide) - If the PR gets final step like approve or lgtm done after 12 hours pass since presubmit tests were run they are getting re-triggered - GitHub things - - EasyCLA sometimes non-properly check the status, it can be re-triggered with /easycla command or it often sorts out when you close and re-open the PR + - EasyCLA sometimes improperly checks the status, it can be re-triggered with `/easycla` command or it often sorts out when you close and re-open the PR From 84f109cd2b413c0d1f64dc2ec257ebde1bcc0a25 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Wed, 14 Aug 2024 22:28:13 +0200 Subject: [PATCH 5/9] Update CONTRIBUTING.md Co-authored-by: Rahul Jadhav --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5be0d8f..22b62d60 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -139,7 +139,7 @@ git commit -s -m 'This is my commit message' ``` #### Code Review of the Pull Request -In Nephio Project uses [Prow](https://docs.prow.k8s.io/) to among many other tasks test and manage the Pull Requests. Prow also allows to interact with PR trough ['slash' commands](https://prow.nephio.io/command-help) +Nephio Project uses [Prow](https://docs.prow.k8s.io/) among many other tools to test and manage the Pull Requests. Prow also allows to interact with PR through ['slash' commands](https://prow.nephio.io/command-help) ##### The Code Review Process The **author** submits a PR From 785fce6666a2d423f4d0462094487c3f7e5c9b90 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Mon, 9 Sep 2024 18:06:14 +0200 Subject: [PATCH 6/9] Update COMMUNITY.md Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> --- COMMUNITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COMMUNITY.md b/COMMUNITY.md index c3210546..bb4839e1 100644 --- a/COMMUNITY.md +++ b/COMMUNITY.md @@ -25,7 +25,7 @@ Members are welcome to remain continuously active contributors to the community. - Ensure affiliation is up to date in [openprofile.dev]. - Have made **several contributions** to the project or community, enough to - demonstrate an **ongoing and possible long-term commitment** to the project like: + demonstrate an **ongoing and possible long-term commitment** to the project such as: - Authoring or reviewing PRs on GitHub, with at least one **merged** PR. - Filing or commenting on issues on GitHub - Contributing to SIG, subproject, or community discussions (meetings, From 2657b4511ac7916ed455d4ddc2e9dcb203099ed1 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Mon, 9 Sep 2024 18:06:28 +0200 Subject: [PATCH 7/9] Update COMMUNITY.md Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> --- COMMUNITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COMMUNITY.md b/COMMUNITY.md index bb4839e1..e25d0d1f 100644 --- a/COMMUNITY.md +++ b/COMMUNITY.md @@ -13,7 +13,7 @@ This document describes the responsibilities and contributor roles in Nephio Pro Anyone is welcome to contribute to Nephio project. Existing members should make a point of being helpful for new contributors in any way possible (PR workflow, finding relevant documentation, guiding with coding style etc.) -Remember we were all '[new contributors]' once. +Remember we were all *new contributors* once. ## Member From 2f99de696abb492386973cdbd9441799d85c9385 Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Mon, 9 Sep 2024 18:06:44 +0200 Subject: [PATCH 8/9] Update COMMUNITY.md Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> --- COMMUNITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COMMUNITY.md b/COMMUNITY.md index e25d0d1f..ac89a75a 100644 --- a/COMMUNITY.md +++ b/COMMUNITY.md @@ -32,7 +32,7 @@ Members are welcome to remain continuously active contributors to the community. Slack, mailing list) - Have read the [contributor guide] - Sponsored by one reviewer -- **Open an issue [Membership request] against the 'nephio' repo** +- **Open an issue **Membership request** against the 'nephio' repo** - Ensure your sponsor is @mentioned on the issue - Provide information described above that is representative of your work on the project - Have your sponsoring reviewer reply confirmation of sponsorship From 1508a70867e227e37a8245090755a65672b3346e Mon Sep 17 00:00:00 2001 From: Rado Chmiel Date: Tue, 10 Sep 2024 18:40:37 +0200 Subject: [PATCH 9/9] Update COMMUNITY.md --- COMMUNITY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/COMMUNITY.md b/COMMUNITY.md index ac89a75a..a7db89af 100644 --- a/COMMUNITY.md +++ b/COMMUNITY.md @@ -103,5 +103,6 @@ compatibility, adhering to API, performance and correctness issues, interactions [new contributors]: /CONTRIBUTING.md +[contributor guide]: /CONTRIBUTING.md [OWNERS]: https://www.kubernetes.dev/docs/guide/owners/ [openprofile.dev]: https://openprofile.dev/edit/profile