mirror of
https://github.com/freedombox/FreedomBox.git
synced 2026-01-21 07:55:00 +00:00
Add contributing guidelines, see issue #964
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>
This commit is contained in:
parent
a1435dd926
commit
57c2835300
110
CONTRIBUTING
Normal file
110
CONTRIBUTING
Normal file
@ -0,0 +1,110 @@
|
||||
Here are some contributing guidelines for authors and reviewers of code changes.
|
||||
The goal is a readable log of code changes, to enhance transparency of their
|
||||
purpose and simplify debugging. Consider these guidelines as best practices, not
|
||||
as absolute rules - we're all learning by doing, and imperfect changes and
|
||||
commits are much better than none at all. For an introduction how to edit and
|
||||
test the code, have look into the HACKING file. Note that you need some basic
|
||||
understanding of Git to contribute; there are many tutorials in the Internet
|
||||
that we cannot repeat here.
|
||||
|
||||
|
||||
Naming conventions:
|
||||
* 'Code change', 'patch', and 'commit' are used interchangeably.
|
||||
* 'Author' and 'contributor' are used interchangeably.
|
||||
* Git 'log' and 'history' are used interchangeably.
|
||||
* PR - pull request
|
||||
* 'Merging' often means 'applying a patch to git history' in a general sense,
|
||||
not literal execution of the command `git merge`.
|
||||
|
||||
|
||||
# For authors of patches
|
||||
* If you would like to contribute, but are unsure what to do, just ask. There
|
||||
are usually also issues tagged as "beginners", which might be a good starting
|
||||
point.
|
||||
* To get your changes included, you must open a pull request (PR), to get them
|
||||
reviewed. Briefly, fork the repository to your account, and edit, commit and
|
||||
push there. Then you can create a PR to the main repository.
|
||||
* Please include one single feature per PR, to keep the review simple and
|
||||
focused on one topic. (This might still mean hundreds of lines of code.) Use
|
||||
another branch than `master`, so you can create multiple PRs and still keep
|
||||
merging from `master`. Depending on the complexity of your PR, it may take a
|
||||
while until it is reviewed and merged.
|
||||
* Please create meaningful commit messages, by following common guidelines:
|
||||
* Multiple lines are allowed if it makes the message clearer.
|
||||
* Separate the first subject line from the text body with a blank line.
|
||||
* Add the component you changed as the first word in the subject line.
|
||||
* Wrap the text at 72 characters.
|
||||
* Use the body to explain what your changes do, and maybe why and how it is
|
||||
achieved (the main idea).
|
||||
* Look into the git log to get an idea.
|
||||
* If it exists, mention the issue number.
|
||||
* End the message with a "Signed-off-by", see next entry.
|
||||
* Consider adding `Signed-off-by: YOUR NAME <YOUR EMAIL>` into your commit
|
||||
message. With this, you explicitly certify that you have the rights to submit
|
||||
your work under the project's license (see LICENSES file) and that you agree
|
||||
to a [Developer Certificate of Origin](http://developercertificate.org/).
|
||||
* If (part of) your code changes were inspired or plainly copied from another
|
||||
source, please indicate this in the PR, so the reviewer can handle it.
|
||||
* Have fun contributing :)
|
||||
|
||||
|
||||
# For reviewers of patches
|
||||
|
||||
## How to review the work of others
|
||||
* Be nice to contributors and give them opportunities to learn. Explain the
|
||||
reasons if you ask for changes instead of silently changing things yourself
|
||||
(unless trivial). This also saves you time in the future.
|
||||
* Reviewers are expected to ensure that a contributor's work:
|
||||
* Does not break the current code. The code base should always be in a
|
||||
usable state, without throwing (non-handled) errors. We also strive to
|
||||
keep the code base in a release-worthy state.
|
||||
* Has no security issues.
|
||||
* Follows coding standards of the project
|
||||
(mainly [PEP-8](https://www.python.org/dev/peps/pep-0008/)).
|
||||
* Is properly internationalized.
|
||||
* Your main job is to make sure that the work runs as expected, by thoroughly
|
||||
testing the patch. New authors are usually not familiar with all areas of
|
||||
impact and may not have tested all cases. It is okay to rely on tests done by
|
||||
trusted authors, if they specify the specific cases they tested.
|
||||
* When merging work from others, add this line to the commit message:
|
||||
`Reviewed-by: YOUR NAME <YOUR EMAIL>`.
|
||||
* Some patches require knowledge of multiple technologies. If you are not
|
||||
familiar with all of them, it's fine to review only the portions you
|
||||
understand (and indicate them clearly). Then ask others for further review.
|
||||
* For major architectural changes/decisions, consult others in the project
|
||||
before merging.
|
||||
* You may make some minimal or obvious changes to the work before merging. If
|
||||
so, tell the contributor (and others) about your edits.
|
||||
* In case more fundamental changes are necessary, or if the contributor is new,
|
||||
try to encourage them to make changes by giving appropriate feedback. This is
|
||||
a major way how we mentor new contributors.
|
||||
* Have fun reviewing :)
|
||||
|
||||
|
||||
## Use of Git
|
||||
* Avoid plain `git merge`, and use rebasing instead, to avoid merge commits.
|
||||
This keeps the history much more readable for others to understand, review,
|
||||
bisect and rollback.
|
||||
* When merging patches with multiple commits, try to make each patch meaningful.
|
||||
For example, fixes to patches should be squashed into the original patch.
|
||||
Security issues and other serious issues should be avoided in intermediate
|
||||
patches – even if they are fixed in later patches.
|
||||
|
||||
|
||||
## Use of GPG
|
||||
* Sign all commits with GPG. This means avoiding GitHub's fancy merge and rebase
|
||||
buttons and doing it locally, where your private key is.
|
||||
* In case a contributor signed with GPG, rebasing will strip it away. To
|
||||
compensate, put your GPG signature on the rebased commits. Given that we have
|
||||
to actually verify the signatures on each commit and the contributor may not
|
||||
be in our web of trust (as we allow anonymous contributions). In such cases,
|
||||
GPG signatures of the reviewers are more important.
|
||||
* Get your GPG key signed by other reviewers or Debian Developers, to enable
|
||||
verification of your signed commits.
|
||||
|
||||
|
||||
## (When) can I commit without review?
|
||||
* Even if you have commit access, you should get your patches reviewed by the
|
||||
other reviewers.
|
||||
* Certain patches do not require review. These include updating the manual, typo
|
||||
fixes, and creating new locales.
|
||||
Loading…
x
Reference in New Issue
Block a user