comaps/docs/PR_GUIDE.md

Pull Request Guidelines

This document gives some guidelines to write and review PR with essential elements.

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
• 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

Review a Pull Request (PR):

• All comments in PR 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
• 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
• 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
• 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 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