From c58a22b299cb634d1f2e7d9b3918b6a61d3d911f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Torrero=20Marijnissen?= Date: Mon, 12 Sep 2022 14:40:02 +0100 Subject: [PATCH 1/3] Improve contribution documentations and add templates --- CONTRIBUTING.md | 59 ++++++++++++++++++++++---- docs/ISSUE_TEMPLATE/bug_report.md | 37 ++++++++++++++++ docs/ISSUE_TEMPLATE/feature_request.md | 20 +++++++++ docs/pull_request_template.md | 31 ++++++++++++++ 4 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 docs/ISSUE_TEMPLATE/bug_report.md create mode 100644 docs/ISSUE_TEMPLATE/feature_request.md create mode 100644 docs/pull_request_template.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff04545104..edff403454 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,15 +1,58 @@ # How to contribute -We're really glad you're reading this, because we need volunteer developers to help this project come to fruition. +Thanks for showing interest and sharing your time to contribute to this project! + +This guide is meant to be used as an overview on how to participate in issues by +creating them in the + -## Submitting changes -Please send a [GitHub Pull Request to Trento](https://github.com/trento-project/web/pull/new/main) with a clear list of what you've done (read more about [pull requests](http://help.github.com/pull-requests/)). +## Opening issues -Always write a clear log message for your commits. One-line messages are fine for small changes, but bigger changes should look like this: +Before creating a new issue make sure you use the search functionality to confirm +that a similar issue doesn't already exist. Next, make sure you properly label +the issue as per our [labels](https://github.com/trento-project/web/labels) +If you are reporting a `bug`, please share a file generated using the +`trento-support.sh` script with the following params: ``` -git commit -m "A brief summary of the commit - -A paragraph describing what changed and its impact." -``` \ No newline at end of file +# trento-support.sh --collect all --output file-tgz +``` +and include the output to your issue. The script should remove sensitive data +automatically but please make sure you are not sharing anything sensible yourself. + +## Submitting changes + +### Pull Requests guideline + +Reviews are hard. This few points will help reduce the effort and allow us to +merge your changes faster. + +1. Only touch relevant files. +2. We have a PR template to aid you in completing the required details. Be + sure to complete it and remove the non-relevant parts. +4. Keep PRs as small as possible. When the PR gets too big, consider splitting + it in multiple parts. An PR should ideally be between 100 and 500 additions. +5. Check that the tests are passing. +6. Commit history should be short and group changes that otherwise wouldn't + make sense on their own. +7. Always write a clear log message for your commits. One-line messages are + fine for small changes, but bigger changes should look like this: + + ``` + git commit -m "A brief summary of the commit + + A paragraph describing what changed and its impact." + ``` +8. Write a detailed description that gives context and explains why you are + creating the PR. +9. If the PR adds functionality, please add some documentation to support it. +10. Each PR needs 1 approval to be merged. Select a reviewer in particular if + you are looking for specific feedback from someone. + +### Reviewers guideline +1. Opinionated comments are welcome but don't expect them always to be + addressed. Be ready for discussion but also open to concede. +2. In order to avoid scope creeping, consider to propose subsequent PRs + rather than requesting changes for the current PR you are reviewing. +3. Short, concise comments with examples are the most valuable. \ No newline at end of file diff --git a/docs/ISSUE_TEMPLATE/bug_report.md b/docs/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000000..f3e54909b3 --- /dev/null +++ b/docs/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,37 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: '' +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Screenshots** +If applicable, add screenshots to help explain your problem. + +**Installation details** + - Use the [trento-support.sh script](https://github.com/trento-project/helm-charts/blob/main/scripts/trento-support.sh) to generate share a file with your installation details, as follows: + + `trento-support.sh` script with the following params: + ``` + # trento-support.sh --collect all --output file-tgz + ``` + - Include the generated file to your issue. The script should remove sensitive data + automatically but please make sure you are not sharing anything sensible yourself. + +**Additional context** +Add any other context about the problem here. diff --git a/docs/ISSUE_TEMPLATE/feature_request.md b/docs/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000000..bbcbbe7d61 --- /dev/null +++ b/docs/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: '' +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. diff --git a/docs/pull_request_template.md b/docs/pull_request_template.md new file mode 100644 index 0000000000..de6e52cf86 --- /dev/null +++ b/docs/pull_request_template.md @@ -0,0 +1,31 @@ +# Description + +Please include a summary of the changes and the related issue, if it exists. +Also, include relevant motivation and context for this PR. List any +dependencies that are required for this change. + +Fixes # (issue) + +## Type of change + +Please delete options that are not relevant. + +- [ ] Bug fix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) +- [ ] This change requires a documentation update + +# How has this been tested? + +Please describe the tests that you wrote to verify your changes. //FIXME: Do we want this? + +# Checklist: + +- [ ] My code follows the style guidelines of this project +- [ ] I have performed a self-review of my code +- [ ] I have commented my code, particularly in hard-to-understand areas +- [ ] I have made corresponding changes to the documentation +- [ ] My changes generate no new warnings +- [ ] I have added tests that prove my fix is effective or that my feature works +- [ ] New and existing unit tests pass locally with my changes +- [ ] Any dependent changes have been merged and published in downstream modules \ No newline at end of file From 8efd1a6cf3da36c4f5cb1a8b06fb8999cc207b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Torrero=20Marijnissen?= Date: Wed, 21 Sep 2022 15:41:46 +0100 Subject: [PATCH 2/3] Fix missing paragraph --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index edff403454..c20dc73e1a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,9 +2,9 @@ Thanks for showing interest and sharing your time to contribute to this project! -This guide is meant to be used as an overview on how to participate in issues by -creating them in the - +This guide is meant to be used as general guideline creating issues or +pull requests. We encourage all first contributors to give this a read to avoid +common mistakes and improve the quality of all contributions. ## Opening issues From 6d0b7bf22b6f81a26cc8eb0d49af110c1501ffc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Torrero=20Marijnissen?= Date: Fri, 23 Sep 2022 15:05:52 +0100 Subject: [PATCH 3/3] Reorganize check list and contributing.md --- CONTRIBUTING.md | 13 ++++++++----- docs/pull_request_template.md | 15 ++------------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c20dc73e1a..2fef3f96b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,9 +34,11 @@ merge your changes faster. 4. Keep PRs as small as possible. When the PR gets too big, consider splitting it in multiple parts. An PR should ideally be between 100 and 500 additions. 5. Check that the tests are passing. -6. Commit history should be short and group changes that otherwise wouldn't +6. Check that your code is not generating new warnings. +7. Check that any dependent changes have been merged and published in downstream modules +8. Commit history should be short and group changes that otherwise wouldn't make sense on their own. -7. Always write a clear log message for your commits. One-line messages are +9. Always write a clear log message for your commits. One-line messages are fine for small changes, but bigger changes should look like this: ``` @@ -44,10 +46,11 @@ merge your changes faster. A paragraph describing what changed and its impact." ``` -8. Write a detailed description that gives context and explains why you are +10. Write a detailed description that gives context and explains why you are creating the PR. -9. If the PR adds functionality, please add some documentation to support it. -10. Each PR needs 1 approval to be merged. Select a reviewer in particular if +11. If the PR adds functionality, please add some tests and documentation + to support it. +12. Each PR needs 1 approval to be merged. Select a reviewer in particular if you are looking for specific feedback from someone. ### Reviewers guideline diff --git a/docs/pull_request_template.md b/docs/pull_request_template.md index de6e52cf86..c7f803a46f 100644 --- a/docs/pull_request_template.md +++ b/docs/pull_request_template.md @@ -15,17 +15,6 @@ Please delete options that are not relevant. - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update -# How has this been tested? +## How to test this? -Please describe the tests that you wrote to verify your changes. //FIXME: Do we want this? - -# Checklist: - -- [ ] My code follows the style guidelines of this project -- [ ] I have performed a self-review of my code -- [ ] I have commented my code, particularly in hard-to-understand areas -- [ ] I have made corresponding changes to the documentation -- [ ] My changes generate no new warnings -- [ ] I have added tests that prove my fix is effective or that my feature works -- [ ] New and existing unit tests pass locally with my changes -- [ ] Any dependent changes have been merged and published in downstream modules \ No newline at end of file +Describe how to test the functionality or the tests that have been added. \ No newline at end of file