mirror of
https://codeberg.org/comaps/comaps
synced 2025-12-19 04:53:36 +00:00
[docs] Simplify documentation to contribute
Signed-off-by: Jean-Baptiste Charron <jeanbaptiste.charron@outlook.fr>
This commit is contained in:
Jean-Baptiste
committed by
Konstantin Pastbin
Konstantin Pastbin
parent
15b3cda4d7
commit
df70ae1b01
@@ -1,75 +0,0 @@
|
||||
# How to write a commit message
|
||||
|
||||
Any commit needs a helpful message. Mind the following guidelines when committing to any of CoMaps repositories at Codeberg.
|
||||
|
||||
1. Separate subject from body with a blank line.
|
||||
2. Limit the subject line to **72 characters**.
|
||||
3. Prefix the subject line with a **subsystem name** in square brackets:
|
||||
|
||||
- [android]
|
||||
- [ios]
|
||||
- [qt]
|
||||
- [search]
|
||||
- [generator]
|
||||
- [strings]
|
||||
- [platform]
|
||||
- [storage]
|
||||
- [transit]
|
||||
- [routing]
|
||||
- [bookmarks]
|
||||
- [3party]
|
||||
- [docs]
|
||||
- ...
|
||||
- See `git log --oneline|egrep -o '\[[0-9a-z]*\]'|sort|uniq -c|sort -nr|less` for ideas.
|
||||
|
||||
4. Start a sentence with a capital letter.
|
||||
5. Do not end the subject line with a period.
|
||||
6. Do not put "gh-xx", "closes #xxx" in the subject line.
|
||||
7. Use the imperative mood in the subject line.
|
||||
|
||||
- A properly formed Git commit subject line should always be able to complete
|
||||
the following sentence: "If applied, this commit will _/your subject line here/_".
|
||||
|
||||
8. Wrap the body to **72 characters** or so.
|
||||
9. Use the body to explain **what and why** vs. how.
|
||||
10. Link Codeberg issues on the last lines:
|
||||
|
||||
- [See tutorial](https://forgejo.org/docs/latest/user/linked-references/).
|
||||
|
||||
11. Use your real name and real email address.
|
||||
|
||||
- See also [Developer's Certificate of Origin](DCO.md)
|
||||
|
||||
A template:
|
||||
|
||||
```
|
||||
[subsystem] Summarize in 72 characters or less
|
||||
|
||||
More detailed explanatory text, if necessary.
|
||||
Wrap it to 72 characters or so.
|
||||
In some contexts, the first line is treated as the subject of the
|
||||
commit, and the rest of the text as the body.
|
||||
The blank line separating the summary from the body is critical
|
||||
(unless you omit the body entirely); various tools like `log`,
|
||||
`shortlog` and `rebase` can get confused if you run the two together.
|
||||
|
||||
Explain the problem that this commit is solving. Focus on why you
|
||||
are making this change as opposed to how (the code explains that).
|
||||
Are there side effects or other unintuitive consequences of this
|
||||
change? Here's the place to explain them.
|
||||
|
||||
Further paragraphs come after blank lines.
|
||||
|
||||
- Bullet points are okay, too.
|
||||
|
||||
- Typically a hyphen or asterisk is used for the bullet, preceded
|
||||
by a single space, with blank lines in between, but conventions
|
||||
vary here.
|
||||
|
||||
Fixes: #123
|
||||
Closes: #456
|
||||
Needed for: #859
|
||||
See also: #343, #789
|
||||
```
|
||||
|
||||
Based on [Tarantool Guidelines](https://www.tarantool.io/en/doc/latest/dev_guide/developer_guidelines/#how-to-write-a-commit-message).
|
||||
@@ -57,7 +57,6 @@ Please [learn how to use `git rebase`](https://git-scm.com/book/en/v2/Git-Branch
|
||||
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.
|
||||
|
||||
- [Pull Request Guide](PR_GUIDE.md).
|
||||
- [How to write a commit message](COMMIT_MESSAGES.md).
|
||||
- [Directories structure](STRUCTURE.md)
|
||||
- [C++ Style Guide](CPP_STYLE.md).
|
||||
- [Java Style Guide](JAVA_STYLE.md).
|
||||
|
||||
@@ -1,46 +1,38 @@
|
||||
# Pull Request Guide
|
||||
# Pull Request Guidelines
|
||||
|
||||
## Writing a good Pull Request (PR):
|
||||
This document gives some guidelines to write and review PR with essential elements.
|
||||
|
||||
- A PR should be small and reflect only one idea, a feature, a bugfix, or a refactoring. In most cases, a PR of 100 lines or less is considered as a PR of normal size and a PR of 1000 lines as big one
|
||||
- If a PR implements two different features, refactoring, or bugfixes, it should be split into several PRs
|
||||
- The description field of every PR should contain a description, links to a ticket or/and links to the confluence
|
||||
- If a PR implements a complex algorithm, the description should contain a link to the proof of the solution or a link to a trusted source
|
||||
- All PRs should contain links to the corresponding tickets. The exception is refactoring if there's nothing to test after it. The reason for that is that every feature or bugfix should be tested by testers after it's implemented
|
||||
- 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 in the same commit
|
||||
- Every commit should reflect a completed idea and have an understandable comment. Review fixes should be merged into one commit
|
||||
## 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
|
||||
- New functionality and unit or integration tests for it should be developed in the same PR
|
||||
- When some source files are changed and then some other source files based on them are auto-generated, they should be committed in different commits. For example, one commit changes strings.txt and another one contains generated files
|
||||
- Most commits and PRs should have prefixes in square brackets depending on the changed subsystem. For example, [routing], [generator], or [android]. Commits and PRs may have several prefixes
|
||||
- A PR which should not be merged after review should be marked as a draft.
|
||||
- All commits and PR captions should be written in English. PR description and PR comments may be written in other languages
|
||||
- All PRs should be approved by at least two reviewers. There are some exceptions: (1) simple PRs to some tools or tests; (2) situation when it's impossible to find another competent reviewer. In any case a PR should be reviewed by at least one reviewer
|
||||
- If somebody left a comment in PR the author should wait for their approval
|
||||
- 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 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.)
|
||||
- Use imperative mood in commit's message, e.g. `[core] Fix gcc warnings` not `[core] Fixed gcc warnings`
|
||||
- When some source files are changed and then some other source files based on them are auto-generated, they should be committed in different commits. For example, if you change style (mapcss) files, then put auto-generated files into a separate [styles] Regenerate commit
|
||||
- All code bases should conform to ./docs/CPP_STYLE.md, ./docs/OBJC_STYLE.md, ./docs/JAVA_STYLE.md or other style in ./docs/ depending on the language
|
||||
- The description field of every PR should contain a description to explain **what and why** vs. how.
|
||||
- If your changes are visual (e.g. app UI or map style changes) then please add before/after screenshots or videos.
|
||||
- Link Codeberg issues into the description field, [See tutorial](https://forgejo.org/docs/latest/user/linked-references/)
|
||||
|
||||
## Doing good code review:
|
||||
## Review a Pull Request (PR):
|
||||
|
||||
- All comments in PR should be polite and concern the PR
|
||||
- If a reviewer criticizes a PR they should suggest a better solution
|
||||
- Reviewer's solution may be rejected by the author if the review did not give arguments why it should be done
|
||||
- Comments in PR should be not only about things which are worth redeveloping but about good designs as well
|
||||
- It's better to ask the developer to make code simpler or add comments to the code base than to understand the code through the developer's explanation
|
||||
- It's worth writing comments in PR which help the PR author to learn something new
|
||||
- All code base should conform to ./docs/CPP_STYLE.md, ./docs/OBJC_STYLE.md, ./docs/JAVA_STYLE.md or other style in ./docs/ depending on the language
|
||||
- Code review should be started as quick as possible after a PR is published. A reviewer should answer developer comments as quick as possible
|
||||
- If a PR looks good to a reviewer they should approve this PR. It means the review is finished
|
||||
- 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
|
||||
- A PR may be merged by a reviewer if all the following items are fulfilled: (1) the PR isn't marked as a draft; (2) all reviewers which have started to review the PR, approved it; (3) all reviewers which are added as reviewers of the PR, have approved it
|
||||
- We prefer PRs to be approved by at least two reviewers. To have a different vision about how the feature/bugs are implemented or fixed, to help to find bugs and test the PR
|
||||
- If a reviewer doesn't have time to review all the PR they should write about it explicitly. For example, LGTM for android part
|
||||
- If a reviewer and a developer cannot find a compromise, a third opinion should be sought
|
||||
- All comments about blank lines should be considered as optional
|
||||
- A PR which should not be merged after review should be marked as a draft.
|
||||
|
||||
## Recommendations:
|
||||
|
||||
- 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
|
||||
- Source files should not be very long
|
||||
- If you are solving a big task it's worth splitting it into subtasks and develop one or several PRs for every subtask.
|
||||
- If it's necessary to make a big change list which should be merged to main at once, it's worth creating a branch and make PRs on it. Then to make PR with all commits of the branch to the main
|
||||
- If you are solving a big task it's worth splitting it into subtasks and developing one or several PRs for every subtask.
|
||||
- In most cases refactoring should be done in a separate PR
|
||||
- If you want to refactor a significant part of the code base, it's worth discussing it with all developers before starting work
|
||||
- If you want to refactor a significant part of the codebase, it's worth discussing it with all developers in an issue before starting work
|
||||
- It's worth using the 'Resolve' conversation button to minimize the list of comments in a PR
|
||||
|
||||
Reference in New Issue
Block a user