-
Notifications
You must be signed in to change notification settings - Fork 22
docs: add CONTRIBUTING.md as a best-practices reference for contributors #92
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| # 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. | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Just one note, I would write 'Apache License, Version 2.0' to unify it with what is written in the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do, thank you |
||
| ## Prerequisites | ||
|
|
||
| Before you begin, ensure you have the following installed: | ||
|
|
||
| - **Go** >= 1.26 (see [go.mod](go.mod)) | ||
| - **make** | ||
| - **tar** | ||
| - **diffutils**, **bzip2**, **gzip** (for tests) | ||
|
Comment on lines
+9
to
+12
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use the same formatting as here. |
||
|
|
||
| Tar-diff is implemented in Go, so familiarity with the language is helpful. | ||
|
|
||
| ## Getting Started | ||
|
|
||
| 1. **Fork the repository** on GitHub | ||
| 2. **Clone your fork** locally: | ||
| ```bash | ||
| git clone https://github.com/YOUR_USERNAME/tar-diff.git | ||
| cd tar-diff | ||
| ``` | ||
| 3. **Add the upstream remote**: | ||
| ```bash | ||
| git remote add upstream https://github.com/containers/tar-diff.git | ||
| ``` | ||
| 4. **Build the project**: | ||
| ```bash | ||
| make | ||
| ``` | ||
|
|
||
| ## Development Workflow | ||
|
|
||
| 1. **Create a new branch** for your work: | ||
| ```bash | ||
| git checkout -b my-feature-branch | ||
| ``` | ||
|
|
||
| 2. **Make your changes** and commit them (see commit guidelines below) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. I would also add a link to the commit guidelines.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a link to conventional commits on line 85, right before going through a list of common commit types. I was thinking we could just leave this mention as plain text |
||
|
|
||
| 3. **Run tests** to ensure everything works: | ||
| ```bash | ||
| make test | ||
| ``` | ||
|
|
||
| 4. **Run validation** to check code quality: | ||
| ```bash | ||
| make validate | ||
| ``` | ||
|
|
||
| 5. **Push your branch** to your fork: | ||
| ```bash | ||
| git push origin my-feature-branch | ||
| ``` | ||
|
|
||
| 6. **Open a Pull Request** against the `main` branch of the upstream repository | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write 'pull request (PR)'. |
||
|
|
||
| ## Sign Your Commits (DCO) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove '(DCO)'. |
||
|
|
||
| All commits must be signed off to certify that you agree to the [Developer Certificate of Origin](https://developercertificate.org/). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write 'Developer Certificate of Origin (DCO)'. |
||
|
|
||
| To sign your commits, use the `-s` flag | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a missing dot. |
||
|
|
||
| This adds a `Signed-off-by` line to your commit message: | ||
| ``` | ||
| Signed-off-by: Your Name <your.email@example.com> | ||
| ``` | ||
|
|
||
| If you forget to sign a commit, you can amend it: | ||
| ```bash | ||
| git commit --amend -s | ||
| ``` | ||
|
|
||
| **Note:** All commits in your PR must be signed off. PRs with unsigned commits will not be merged. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write 'Note:' to unify it with other similar occurrences. |
||
|
|
||
| ## Commit Signature Verification | ||
|
|
||
| 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). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write 'see GitHub's documentation on commit signature verification'. |
||
|
|
||
| ## Git Commit Style | ||
|
|
||
| This project follows the [Conventional Commits](https://www.conventionalcommits.org/) specification. | ||
|
|
||
| ### Commit Message Format | ||
|
|
||
| ``` | ||
| <type>: <description> | ||
| [optional body] | ||
| [optional footer(s)] | ||
| Signed-off-by: Your Name <your.email@example.com> | ||
| ``` | ||
|
|
||
| ### Commit Types | ||
|
|
||
| - **feat**: A new feature | ||
| - **fix**: A bug fix | ||
| - **docs**: Documentation changes | ||
| - **test**: Adding or updating tests | ||
| - **ci**: Changes to CI/CD workflows | ||
| - **refactor**: Code refactoring without changing functionality | ||
| - **perf**: Performance improvements | ||
| - **chore**: Maintenance tasks, dependency updates | ||
|
|
||
|
|
||
| ## Testing | ||
|
|
||
| Before submitting a PR, ensure all local tests pass: | ||
|
|
||
| ### Run all tests: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a consistent heading style for '###' titles - for example, '### Commit Types' vs. '### Run all tests:'. |
||
| ```bash | ||
| make test | ||
| ``` | ||
|
|
||
| ### Run unit tests only: | ||
| ```bash | ||
| make unit-test | ||
| ``` | ||
|
|
||
| ### Run integration tests only: | ||
| ```bash | ||
| make integration-test | ||
| ``` | ||
|
|
||
| ### Check test coverage: | ||
| Test coverage reports are generated in `test/coverage/` after running tests. | ||
|
|
||
| ## Code Quality | ||
|
|
||
| ### Format your code: | ||
| ```bash | ||
| make fmt | ||
| ``` | ||
|
|
||
| ### Run linting and validation: | ||
| ```bash | ||
| make validate | ||
| ``` | ||
|
|
||
| This runs: | ||
| - `golangci-lint` for linting | ||
| - `go vet` for static analysis | ||
| - `gofmt` check for formatting | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would skip 'check'. |
||
|
|
||
| All validation checks must pass before your PR can be merged. | ||
|
|
||
| ## Cross-Platform Compatibility | ||
|
|
||
| Tar-diff supports Linux, macOS, and Windows. When making changes: | ||
|
|
||
| - Avoid platform-specific code unless absolutely necessary | ||
| - Test on multiple platforms when possible | ||
| - The CI pipeline will test your changes on all supported platforms | ||
|
|
||
| ## Pull Request Process | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The abbreviation 'PR' should be introduced earlier so that 'PR Process' can be used as the heading. |
||
|
|
||
| 1. **Ensure your PR**: | ||
| - Has a clear description of what it does and why | ||
| - References any related issues (see below) | ||
| - Passes all CI checks | ||
| - Has all commits signed off (DCO) | ||
| - Follows the commit message conventions | ||
|
|
||
| 2. **Link PRs to Issues**: | ||
| If your PR addresses an existing issue, link it in the PR description using GitHub keywords: | ||
| - `Fixes #123`, `Closes #123`, `Resolves #123` - Automatically closes the issue when the PR is merged | ||
| - `Related to #123`, `Addresses #123` - Links the PR without auto-closing | ||
|
|
||
| 3. **Code review**: | ||
| - 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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We did some modifications to the comment resolution (it is not so strict). It should maybe be incorporated here. |
||
| - **Approvals required**: PRs require at least **2 approvals** from maintainers before they can be merged | ||
|
|
||
| 4. **After approval**: | ||
| - Once your PR has the required approvals and all comments are resolved, a maintainer will merge your PR | ||
|
|
||
| ## Merging and Squashing | ||
|
|
||
| This project values **clean, readable git history**. Code is read more often than it's written, so maintaining meaningful history for tools like `git log`, `git blame`, and `git revert` is important. | ||
|
|
||
| - **Use merge commits** (default) when the PR contains clean, individual, well-structured commits that each serve a purpose | ||
| - **Use squash merge** only when the PR contains fixup commits, debug iterations, or review feedback that doesn't need to be preserved in history | ||
|
|
||
|
|
||
| ## Questions or Issues? | ||
|
|
||
| If you have questions or run into issues: | ||
|
|
||
| - Check existing [issues](https://github.com/containers/tar-diff/issues) | ||
| - Open a new issue if needed | ||
| - Feel free to ask questions in your PR | ||
|
|
||
| Thank you for contributing to tar-diff :)! | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,10 @@ Delta compression is based on [bsdiff](http://www.daemonology.net/bsdiff/) and [ | |
|
|
||
| The `tar-diff` file format is described in [file-format.md](file-format.md). | ||
|
|
||
| ## Contributing to tar-diff | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write |
||
|
|
||
| Interested in collaborating on tar-diff? Check out our [CONTRIBUTING.md](CONTRIBUTING.md) guide to help get you started! | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write |
||
|
|
||
| ## License | ||
|
|
||
| `tar-diff` is licensed under the Apache License, Version 2.0. See | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would write 'Contributing to
tar-diff'. It means that I would replace 'Tar-diff' withtar-diffin general.