-
Notifications
You must be signed in to change notification settings - Fork 7
Reviewing
mkind edited this page Mar 1, 2016
·
1 revision
- We use feature branches named
YYYY-MM-<dev>-<feature>
where is the developer and a string describing the feature implemented. - Keep branches clean and short. They should only cover ONE semantic aspect.
- Keep commits clean and short. Like branches, each commit should cover ONE semantic aspect.
- If a commit has to be reverted, do not recommit the old changes. Instead use
git revert
. - There are slides that give an introduction to our git policy. Just ask.
- In order to get something merged into
master
, you must ask a developer to review your code. After reviewing and discussing your code, it can be merged. There should be no I-need-this-to-be-done-fast-so-I-review-myself-and-merge-it-quickly-shit. It is totally ok, if reviewing takes some time. Keep in mind, that the reviewer needs to understand your code. - see Coding Conventions.
-
git pull
and checkout the branch to review. - Check for typical issues, discussed in the CERT Secure Coding Style (mem handling, if conditions). Maybe we should list some more examples. This step is important, so be pedantic!
- build with
make EXTRA_FLAGS="-DCLANG_ADDRESS_SANITIZER=ON -DCLANG_ANALYZER=ON"
(and other sanatizer) - read output and check for warnings and errors. In the best case, there are no warnings and errors.
- Every test must succeed before merging into
master
.
- valgrind
- flawfinder