docs: add CONTRIBUTING.md as a best-practices reference for contributors#92
docs: add CONTRIBUTING.md as a best-practices reference for contributors#92rmiki-dev wants to merge 1 commit intocontainers:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive CONTRIBUTING.md file outlining the project's development workflow, commit standards, and testing procedures. Feedback was provided to correct a typo and remove trailing whitespace in the introduction, as well as to update the minimum Go version requirement to 1.26 to align with the project's go.mod file.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
+ Coverage 72.87% 79.37% +6.49%
==========================================
Files 10 10
Lines 1412 1115 -297
==========================================
- Hits 1029 885 -144
+ Misses 271 133 -138
+ Partials 112 97 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1e65cf6 to
2339171
Compare
Signed-off-by: Rosy-Glorious Miki <rmiki@redhat.com>
2339171 to
2faade9
Compare
kgiusti
left a comment
There was a problem hiding this comment.
Thanks for adding this! I had a couple of recommendations for you to consider - see inline.
| git checkout -b my-feature-branch | ||
| ``` | ||
|
|
||
| 2. **Make your changes** and commit them (see commit guidelines below) |
There was a problem hiding this comment.
I would move the commit part of the sentence to after steps 3 and 4, and emphasize that all the tests and validation (steps 3 and 4) must pass before the commit is created.
There was a problem hiding this comment.
I agree. I would also add a link to the commit guidelines.
| # Contributing to Tar-diff | ||
|
|
||
| Thank you for your interest in Tar-diff! We appreciate contributions that help improve this library and maintain high code quality. This guide covers everything you need to know to contribute effectively to the project. | ||
|
|
There was a problem hiding this comment.
I would recommend adding a Note: here about the licensing requirement. Something like:
This project is licensed under the Apache License 2.0. By contributing, you agree that your contributions will be licensed under the same terms.
There was a problem hiding this comment.
I agree. Just one note, I would write 'Apache License, Version 2.0' to unify it with what is written in the README.md file.
| @@ -0,0 +1,200 @@ | |||
| # Contributing to Tar-diff | |||
There was a problem hiding this comment.
I would write 'Contributing to tar-diff'. It means that I would replace 'Tar-diff' with tar-diff in general.
| - **Go** >= 1.26 (see [go.mod](go.mod)) | ||
| - **make** | ||
| - **tar** | ||
| - **diffutils**, **bzip2**, **gzip** (for tests) |
| git push origin my-feature-branch | ||
| ``` | ||
|
|
||
| 6. **Open a Pull Request** against the `main` branch of the upstream repository |
There was a problem hiding this comment.
I would write 'pull request (PR)'.
|
|
||
| 6. **Open a Pull Request** against the `main` branch of the upstream repository | ||
|
|
||
| ## Sign Your Commits (DCO) |
|
|
||
| ## Sign Your Commits (DCO) | ||
|
|
||
| All commits must be signed off to certify that you agree to the [Developer Certificate of Origin](https://developercertificate.org/). |
There was a problem hiding this comment.
I would write 'Developer Certificate of Origin (DCO)'.
|
|
||
| All commits must be signed off to certify that you agree to the [Developer Certificate of Origin](https://developercertificate.org/). | ||
|
|
||
| To sign your commits, use the `-s` flag |
|
|
||
| All commits must have **verified signatures** (shown as a "Verified" badge on GitHub). This is enforced by branch protection rules - commits without verified signatures will be rejected. | ||
|
|
||
| To set up commit signing, see [GitHub's documentation on commit signature verification](https://docs.github.com/en/authentication/managing-commit-signature-verification). |
There was a problem hiding this comment.
I would write 'see GitHub's documentation on commit signature verification'.
|
|
||
| Before submitting a PR, ensure all local tests pass: | ||
|
|
||
| ### Run all tests: |
There was a problem hiding this comment.
Consider using a consistent heading style for '###' titles - for example, '### Commit Types' vs. '### Run all tests:'.
| This runs: | ||
| - `golangci-lint` for linting | ||
| - `go vet` for static analysis | ||
| - `gofmt` check for formatting |
| - Test on multiple platforms when possible | ||
| - The CI pipeline will test your changes on all supported platforms | ||
|
|
||
| ## Pull Request Process |
There was a problem hiding this comment.
The abbreviation 'PR' should be introduced earlier so that 'PR Process' can be used as the heading.
| - Maintainers will review your PR | ||
| - Address any feedback or requested changes | ||
| - Keep your branch up to date with `main` to minimize merge conflicts | ||
| - **Comment resolution**: The person who opens a review comment is responsible for resolving it once the feedback has been addressed |
There was a problem hiding this comment.
We did some modifications to the comment resolution (it is not so strict). It should maybe be incorporated here.
| git commit --amend -s | ||
| ``` | ||
|
|
||
| **Note:** All commits in your PR must be signed off. PRs with unsigned commits will not be merged. |
There was a problem hiding this comment.
I would write 'Note:' to unify it with other similar occurrences.
| - Open a new issue if needed | ||
| - Feel free to ask questions in your PR | ||
|
|
||
| Thank you for contributing to tar-diff :)! |
|
|
||
| The `tar-diff` file format is described in [file-format.md](file-format.md). | ||
|
|
||
| ## Contributing to tar-diff |
|
|
||
| ## Contributing to tar-diff | ||
|
|
||
| Interested in collaborating on tar-diff? Check out our [CONTRIBUTING.md](CONTRIBUTING.md) guide to help get you started! |
| This is particularly useful for e.g. bootc images, where only the files in the ostree repo | ||
| will be available on the system. For that case you would run tar-diff with | ||
| `--source-prefix=sysroot/ostree/repo/objects/` |
There was a problem hiding this comment.
I would write 'This is particularly useful for bootc images, for example, where only the files in the ostree repository will be available on the system. In that case, you would run tar-diff with --source-prefix=sysroot/ostree/repo/objects/.'.
I know it is not related to the changes in this PR. I just wanted to share this suggestion with you.
knecasov
left a comment
There was a problem hiding this comment.
I added several comments.
CONTRIBUTING.md file to guide new contributors through the development workflow, commit requirements, and PR process for the tar-diff project.
A "Contributing to tar-diff" section has also been added to
README.mddirecting users to the guideThe key sections in this PR include:
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com