diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 7c9358727..19abaf657 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -53,7 +53,7 @@ Sometimes it's better to discuss and confirm your vision of the fix or implement Please [learn how to use `git rebase`](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) (or rebase via any git tool with a graphical interface, e.g. [Fork for Mac](https://git-fork.com/) is quite good) to amend your commits in the PR and maintain a clean logical commit history for your changes/branches. -While we strive to help onboard new developers we don't have enough time to guide everyone step-by-step and explain in detail how everything works (in many cases we have to study the code ourselves). You'll need to be largely self-sufficient, expect to read a lot of code and documentation. +We strive to help onboard new developers but we don't always have enough time to guide newcomers step-by-step and explain everything in detail. For that reason we might ask you to read lots of the documentation and study the existing code. - [Pull Request Guide](PR_GUIDE.md). - [Directories structure](STRUCTURE.md) diff --git a/docs/PR_GUIDE.md b/docs/PR_GUIDE.md index 60bc9d0cf..cc331a9d6 100644 --- a/docs/PR_GUIDE.md +++ b/docs/PR_GUIDE.md @@ -4,10 +4,10 @@ This document gives some guidelines to write and review PR with essential elemen ## Writing a Pull Request (PR): -- A PR should have a reasonable size and reflect only one idea, a feature, a bugfix, or a refactoring, to help review and limit regression in the app -- If a PR implements two different features, refactoring, or bug fixes, we prefer you to split it into several PRs +- A PR should have a reasonable size and reflect only one idea, a feature, a bugfix, or a refactoring. This helps to review and limit regressions in the app +- If you want to work on more than one feature/bug/refactoring at once, please separate PRs for these. - New functionality and unit or integration tests for it should be developed in the same PR -- Every commit of all PRs should be compilable under all platforms. All tests should pass. So if changing of code breaks unit or integration tests these tests should be fixed +- Every commit of all PRs should be compilable under all platforms and all tests should pass. If changes break unit or integration tests, then these tests should be fixed, ideally before opening a PR. - Every commit should reflect a completed idea and have an understandable comment. Review fixes should be merged into one commit - All commits and PR captions should be written in English. - We suggest PRs should have prefixes in square brackets depending on the changed subsystem. For example, [routing], [generator], or [android]. Commits may have several prefixes (See `git log --oneline|egrep -o '\[[0-9a-z]*\]'|sort|uniq -c|sort -nr|less` for ideas.) @@ -20,7 +20,7 @@ This document gives some guidelines to write and review PR with essential elemen ## Review a Pull Request (PR): -- All comments in PR should be polite and concern the PR +- All review comments should be polite and concern the PR - It's better to ask the developer to make the code simpler or add comments to the codebase than to understand the code through the developer's explanation - If a developer changes the PR significantly, the reviewers who have already approved the PR should be informed about these changes - A reviewer should pay attention not only to the code base but also to the description of the PR and commits @@ -29,7 +29,36 @@ This document gives some guidelines to write and review PR with essential elemen - If a reviewer and a developer cannot find a compromise, a third opinion should be sought - A PR which should not be merged after review should be marked as a draft. -## Recommendations: +### Reviewing pull requests of first-time contributors + +First-time contributors are likely to be less familiar with both the project code but also the way CoMaps as a community operates. +Some of them might also not be too familiar with using git (e.g. for rebasing branches or signing DCOs). + +For those reasons they might require more effort from reviewers. +At the same time, new contributors are the lifeblood that keeps CoMaps sustainable by ensuring that enough people are contributing code. For that reason, it's important to help those newcomers in their on-boarding. + +To help newcomers in feeling welcome and getting onboarded, some suggestions for what to include in : + +- Thank the contributor for opening a PR (they volunteered time to help the project! And offering code to a new community can be nerve-wrecking.) +- Where applicable: + * Notes on how to sign the DCO/rebase commits + * The necessary constructive feedback if there are things that need to be improved before a PR can be merged +- Give the contributor another thank you for their effort, signal that the community is open for helping further (e.g. on Codeberg and link to Matrix/TG/Zulip) + +Here's a small template that can help writing feedback to first-time contributors: + +> Thank you so much for this contribution - and welcome to CoMaps! +> +> For CoMaps, we use the [_Developers Certficate of Origin_](https://codeberg.org/comaps/comaps/src/branch/main/docs/DCO.md) for contributors to certify that they wrote/have the right to submit their code to our project. This helps ensure that CoMaps can remain open source as it gives us some legal protection. You can sign it by signing your commit. The easiest way is to amend your existing commit with `git commit -s -m 'your commit message'`. If you are using the Codeberg interface you can also sign your commit by selection _"Add a Signed-off-by trailer by the committer at the end of the commit log message."_. +> +> There are a few things that could help improve your PR/would need to be fixed / I have a few questions about… (the actual PR feedback) +> +> I hope that helps! If anything is unclear, please don't hesitate to get in touch on here, or reach out via our chat channels. You can find most of us [on different Matrix/Telegram channels](https://codeberg.org/comaps/Governance/wiki/Chat-rooms) or [on our contributor Zulip](https://comaps.zulipchat.com/join/e5e3c4zurmfxykrtbiiq6qrw/). +> +> Thanks again for putting this PR together! + + +## Recommendations for making PRs: - Functions and methods should not be long. In most cases, it's good if the whole body of a function or method fits on the monitor. It's good to write a function or a method shorter than 60 lines - If you are solving a big task it's worth splitting it into subtasks and developing one or several PRs for every subtask.