|
|
If we are in a position where new developments or bug fixes need to be merged from a branch back to master, follow the rules defined here. If there is anything unclear also in specific cases, we can always discuss in our meetings.
|
|
|
|
|
|
- Only Merge Requests (MR) with the Tag "ready for code review", and without WIP status are reviewed
|
|
|
- Make sure the CI jobs all run fine
|
|
|
- Also run the extra optional jobs "coverage", "release-clang-8", "release-u-18.04" and make sure no problems occur
|
|
|
- Check in the "coverage" job output that the coverage did not decline (!). It should always stay, or increase.
|
|
|
- Make sure clang-format was used, if necessary by checking out the branch and running `./do-clang-format.py` and check for any reported file. If there are files:
|
|
|
- Ask the author of the MR to run `do-clang-format.py --apply`
|
|
|
- Make sure the copyright headers are there. If necessary by checking out the branch and running `./do-copyright.py --check`. If there are problems:
|
|
|
- Ask the author of the MR to fix
|
|
|
- From the MR page, open the "Open in Web IDE" tool
|
|
|
- Check all changes for coding rules and guidelines
|
|
|
- Check if the provided solution solves the Issue, discuss on gitlab
|
|
|
- If useful discuss ongoing MR review in CORSIKA meeting
|
|
|
- Approve on gitlab
|
|
|
|
|
|
```mermaid
|
|
|
graph TD;
|
|
|
subgraph "Code contributor"
|
... | ... | @@ -24,29 +10,28 @@ graph TD; |
|
|
end
|
|
|
A4 --> B0[A suited developer is picked for code review];
|
|
|
B0 --> C1[Reviewer copies task list from below into MR];
|
|
|
|
|
|
subgraph "Code reviewer"
|
|
|
C1 --> C2[Reviewer checks each item on task list];
|
|
|
C2 --> C3[Final check means APPROVED for merging];
|
|
|
C2 --> C3[Final check mark means APPROVED for merging];
|
|
|
end
|
|
|
C3 --> D[Developer merges code into master, closed MR]
|
|
|
C3 --> D[Developer merges code into master, closes MR]
|
|
|
```
|
|
|
|
|
|
Until all boxes below are checked the reviewer gets back to the contributor and asks for specific fixes. If there are larger issues either the reviewer or the contributor can both discuss this also in CORSIKA meetings or via the corsika-devel mailing list.
|
|
|
|
|
|
Concrete rules are listed here. Make a copy of the below section in the MR discussion section and go through all the points:
|
|
|
```
|
|
|
- [] The MR has the Tag "ready for code review", and is without WIP status
|
|
|
- [] Make sure the most recent CI jobs (config, quality, build, tests) all run fine
|
|
|
- [] The MR has the Tag "ready for code review", and is without WIP (work in progress) status
|
|
|
- [] Make sure the most recent CI jobs (config, quality, build, tests) all run fine with no failures
|
|
|
- if "check clang-format" failed: the code contributor has to run `./do-clang-format.py --apply` eventually with the `--all` option
|
|
|
- if "check copyright" failed the code contributor has to run`./do-copyright.py --add=20xy`
|
|
|
- [] Also run the extra optional jobs "coverage", "release-clang-8", "release-u-18.04" and make sure no problems occur
|
|
|
- [] Check in the "coverage" job output that the coverage did not decreased (!). It should always stay, or increase.
|
|
|
- [] Make sure clang-format was used, if necessary by checking out the branch and running `./do-clang-format.py` and check for any reported file. If there are files:
|
|
|
- [] Ask the author of the MR to run `do-clang-format.py --apply`
|
|
|
- [] Make sure the copyright headers are there. If necessary by checking out the branch and running `./do-copyright.py --check`. If there are problems:
|
|
|
- [] Ask the author of the MR to fix
|
|
|
- [] From the MR page, open the "Open in Web IDE" tool
|
|
|
- [] Check all changes for coding rules and guidelines
|
|
|
- [] Check if the provided solution solves the Issue, discuss on gitlab
|
|
|
- [] If useful discuss ongoing MR review in CORSIKA meeting
|
|
|
- [] Approve on gitlab
|
|
|
- [] Check in the "coverage" job output that the coverage did not decreased (!). It should always stay, or increase. If it decreased --> add further unit tests, and check coverage report.
|
|
|
- On the MR page, open the "Open in Web IDE" tool
|
|
|
- [] Check if the provided solution solves the Issue, discuss on gitlab
|
|
|
- [] Check that all changes are actually related to the issue
|
|
|
- [] There are no debug statements left, not even commented out
|
|
|
- [] Check all changes for coding rules and guidelines
|
|
|
- [] **Click here to approve this MR for merging**
|
|
|
``` |