From 7f37255ad610cd9f42520a49e15687c1ece30be7 Mon Sep 17 00:00:00 2001 From: Anna Kapuscinska Date: Wed, 29 May 2024 21:32:50 +0100 Subject: [PATCH] docs: Refactor Contribution Guide Tetragon contribution guide consists of a few docs pages and a PR template, each one containing a list of steps. Many of the steps (e.g. testing or commit message guidelines) are duplicated between lists. This is hard to follow and hard to maintain (different pages can easily become conflicting). I refactored the contribution guide so that it's (hopefully) easier to follow: * "Making changes" page has simplified build & test instructions. I added a step about code generation and removed a step about commits. * "Submitting a pull request" page covers everything related to commits (in a dedicated section) and basic info about pull request review. I added a link to the main contribution guide and removed other steps, which were duplicating the previous guides * Checklist in the PR template is simplified and links to docs pages. Signed-off-by: Anna Kapuscinska --- .github/pull_request_template.md | 14 ++-- .../docs/contribution-guide/making-changes.md | 29 +++---- .../submitting-a-pull-request.md | 78 ++++++++++--------- 3 files changed, 56 insertions(+), 65 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2ade8e03245..cc50380889c 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,14 +1,10 @@ -Please ensure your pull request adheres to the following guidelines: +Thanks for contributing! Please ensure your pull request adheres to the following guidelines: -- [ ] For first time contributors, read [Submitting a pull request](https://tetragon.io/docs/contribution-guide/submitting-a-pull-request/). -- [ ] All code is covered by unit and/or end-to-end tests tests where feasible. -- [ ] All commits contain a well written commit message including a title, - description and a `Fixes: #XXX` line if the commit addresses a particular - GitHub issue. -- [ ] All commits are signed off. See the section [Developer’s Certificate of Origin](https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#dev-coo) +- [ ] All commits contain a well written commit message and are signed-off (see [Submitting a pull request](https://tetragon.io/docs/contribution-guide/submitting-a-pull-request/)). +- [ ] All code is covered by unit and/or end-to-end tests where feasible. +- [ ] All generated files are updated if needed (see [Making changes](https://tetragon.io/docs/contribution-guide/making-changes/)). - [ ] Provide a title or release-note blurb suitable for the release notes. -- [ ] Are you a user of Tetragon? Please add yourself to the [Users doc](https://github.com/cilium/cilium/blob/main/USERS.md) in the Cilium repository. -- [ ] Thanks for contributing! +- [ ] Are you a user of Tetragon? Please add yourself to the [Users doc](https://github.com/cilium/cilium/blob/main/USERS.md) in the Cilium repository. diff --git a/docs/content/en/docs/contribution-guide/making-changes.md b/docs/content/en/docs/contribution-guide/making-changes.md index 06c965e2d8e..d1ad9f4ebb0 100644 --- a/docs/content/en/docs/contribution-guide/making-changes.md +++ b/docs/content/en/docs/contribution-guide/making-changes.md @@ -24,29 +24,20 @@ description: "Learn how to make your first changes to the project" 3. Make the changes you want. -4. Separate the changes into logical commits. +4. Test your changes. Follow [Development setup](/contribution-guide/development-setup) and + [Running tests](/docs/contribution-guide/running-tests) guides to build and test Tetragon. - - Describe the changes in the commit messages. Focus on answering the - question why the change is required and document anything that might be - unexpected. - - If any description is required to understand your code changes, then those - instructions should be code comments instead of statements in the commit - description. - - For submitting PRs, all commits need to be signed off `(git commit -s)`. - See the section [Developer's Certificate of Origin]({{< ref "/docs/contribution-guide/developer-certificate-of-origin" >}}) + - Make sure that all new code is covered by unit and/or end-to-end tests where feasible. + - Run Tetragon locally to validate everything works as expected. + - If adding/extending tests is not required, mention in the commit message what existing test covers the new code + or how you validated the change. -5. Make sure your changes meet the following criteria: +5. Run code/docs generation commands if needed (see the sections below for specific code areas). - - New code is covered by Integration Testing. - - End to end integration / runtime tests have been extended or added. If not - required, mention in the commit message what existing test covers the new - code. - - Follow-up commits are squashed together nicely. Commits should separate - logical chunks of code and not represent a chronological list of changes. +6. Run `git diff --check` to catch obvious white space violations. -6. Run `git diff --check` to catch obvious white space violations - -7. Build Tetragon with your changes included. +7. Follow [Submitting a pull request](/docs/contribution-guide/submitting-a-pull-request) guide to commit your changes + and open a pull request. ## Making changes to documentation diff --git a/docs/content/en/docs/contribution-guide/submitting-a-pull-request.md b/docs/content/en/docs/contribution-guide/submitting-a-pull-request.md index 33dfdce34af..61b25e1291a 100644 --- a/docs/content/en/docs/contribution-guide/submitting-a-pull-request.md +++ b/docs/content/en/docs/contribution-guide/submitting-a-pull-request.md @@ -4,66 +4,70 @@ weight: 5 description: "Learn how to submit a pull request to the project" --- -### Submitting a pull request +{{< note >}} +This guide assumes that you have already made and tested changes you want to contribute. If you have not, +please follow the steps from the [Contribution Guide]({{< ref "/docs/contribution-guide#guide-for-code-and-docs-contribution" >}}). +{{< /note >}} -Contributions must be submitted in the form of pull requests against the -upstream GitHub repository at https://github.com/cilium/tetragon. - -1. Fork the Tetragon repository. +## Commit changes -2. Push your changes to the topic branch in your fork of the repository. +Save your changes in one or more commits. Commits should separate logical chunks of code and not represent +a chronological list of changes. Ideally, each commit compiles and is functional on its own to allow for bisecting. -3. Submit a pull request on https://github.com/cilium/tetragon. +If in code review you are requested to make changes, squash the follow-up changes into the existing commits. +If you are not comfortable with Git yet (in particular with `git rebase`), refer to the [GitHub documentation](https://docs.github.com/en/get-started/using-git/using-git-rebase-on-the-command-line). -Before hitting the submit button, please make sure that the following -requirements have been met: +### Write a commit message -1. Each commit compiles and is functional on its own to allow for bisecting of - commits. +All commits must contain a well-written commit message: -2. All code is covered by unit and/or runtime tests where feasible. +1. Write a title, **no longer than 75 characters**. If the commit covers one specific area, start the title with + a prefix like `helm:` or `metrics:`. +2. Describe the changes in the commit description. Focus on answering the question why the change is required and + document anything that might be unexpected. If any explanation is required to understand your code, then it should + be written in the code comments instead of the commit description. +3. Add a `Fixes: #XXX` line if the commit addresses a particular GitHub issue identified by its number. Note that the + GitHub issue will be automatically closed when the commit is merged. +4. All commits **must be signed off** `(git commit -s)`. + See the section [Developer's Certificate of Origin]({{< ref "/docs/contribution-guide/developer-certificate-of-origin" >}}). -3. All changes have been tested and checked for regressions by running the - existing testsuite against your changes. +### Example commit message -4. All commits contain a well written commit description including a title, - description and a `Fixes: #XXX` line if the commit addresses a particular - GitHub issue identified by its number. Note that the GitHub issue will be - automatically closed when the commit is merged. +``` +doc: add contribution guideline and how to submit pull requests - ``` - doc: add contribution guideline and how to submit pull requests +Tetragon Open Source project was just released and it does not include +default contributing guidelines. - Tetragon Open Source project was just released and it does not include - default contributing guidelines. +This patch fixes this by adding: - This patch fixes this by adding: +1. CONTRIBUTING.md file in the root directory as suggested by github documentation: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors - 1. CONTRIBUTING.md file in the root directory as suggested by github documentation: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors +2. Development guide under docs directory with a section on how to submit pull requests. - 2. Development guide under docs directory with a section on how to submit pull requests. +3. Moves the DEVELOP.md file from root directory to the `docs/contributing/development/` one. - 3. Moves the DEVELOP.md file from root directory to the `docs/contributing/development/` one. +Fixes: #33 - Fixes: #33 +Signed-off-by: Djalal Harouni +``` - Signed-off-by: Djalal Harouni - ``` +## Submit a pull request - Note: Make sure to include a blank line in between commit title and commit description. - -5. All commits are signed off. See the section [Developer’s Certificate of - Origin]({{< ref "/docs/contribution-guide/developer-certificate-of-origin" >}}). +Contributions must be submitted in the form of pull requests against the +upstream GitHub repository at https://github.com/cilium/tetragon. -6. All important steps in [Making changes]({{< ref "/docs/contribution-guide/making-changes" >}}) have been followed. +Please follow the checklist in the pull request template and write anything that reviewers should be aware of in the +pull request description. After you create a pull request, a reviewer will be automatically assigned. They will provide +feedback, add relevant labels and run the CI workflows if needed. -### Frequently Asked Questions +## Frequently Asked Questions -#### CI is complaining about Go module vendoring, what do I do? +### CI is complaining about Go module vendoring, what do I do? You can run `make vendor` then add and commit your changes. -#### CI is complaining about a missing "signed-off-by" line. What do I do? +### CI is complaining about a missing "signed-off-by" line. What do I do? You need to add a signed-off-by line to your commit messages. The easiest way to do this is with `git fetch origin/main && git rebase --signoff origin/main`.