About task 4
You are to keep in mind many things:
Don’t use a check that has been used by someone else
#3009 , #3011 and #3013
are using the same check cppcoreguidelines-init-variables
You will be doing double work:
Suppose that #3009 finishes all the fixes for that check. the PR will be merged and when you rebase (read bellow about keeping your branch up to date) the changes you did were useless, because the problem do not exist any more.
We need to fix everything:
git checkout upstream/develop
Edit .clang-tidy
more .clang-tidy
Checks: >
google-*,
-google-readability-casting,
clang-analyzer-*,
clang-diagnostic-*,
cppcoreguidelines-*
WarningsAsErrors: ''
HeaderFilterRegex: './include'
FormatStyle: none
InheritParentConfig: true
WarningsAsErrors: ''
HeaderFilterRegex: './include'
FormatStyle: none
InheritParentConfig: true
Note in my run.sh I have:
CXX=clang++ CC=clang cmake -DUSE_CLANG_TIDY=ON -DBUILD_HTML=OFF -DPOSTGRESQL_BIN=${PGBIN} ..
I execute my run.sh
rm -rf build/* ; bash run.sh &>> get-problems.txt
and get the problems
grep cppcore get-problems.txt | awk -F 'cppcore' '{print "cppcore"$2}' | sort | uniq
cppcoreguidelines-avoid-c-arrays]
cppcoreguidelines-avoid-const-or-ref-data-members]
cppcoreguidelines-avoid-do-while]
cppcoreguidelines-avoid-magic-numbers]
cppcoreguidelines-explicit-virtual-functions]
cppcoreguidelines-init-variables]
cppcoreguidelines-macro-usage]
cppcoreguidelines-narrowing-conversions]
cppcoreguidelines-no-malloc]
cppcoreguidelines-non-private-member-variables-in-classes]
cppcoreguidelines-owning-memory]
cppcoreguidelines-prefer-member-initializer]
cppcoreguidelines-pro-bounds-array-to-pointer-decay]
cppcoreguidelines-pro-bounds-pointer-arithmetic]
cppcoreguidelines-pro-type-const-cast]
cppcoreguidelines-pro-type-member-init]
cppcoreguidelines-pro-type-reinterpret-cast]
cppcoreguidelines-pro-type-union-access]
cppcoreguidelines-pro-type-vararg]
cppcoreguidelines-slicing]
cppcoreguidelines-special-member-functions]
cppcoreguidelines-use-default-member-init]
cppcoreguidelines-virtual-class-destructor]
So watch what others are doing so you do not do something that might be merged
First come first served
I am reviewing the Pull requests in order, first #3003 then #3004 then #3005 etc …
In the review 2 things might happen
Work requested
When you have work requested: see what is asked.
The majority either:
- Did not fix a warning
- Maybe you could not figure out how to compile locally to see the warnings
- Maybe you could not figure out where to look on the CI to see the warnings
- Maybe its work pending
- Fixed only one warning
The task is to fix all the warnings generated because of the new check.
- Some of the checks generate 30 warnings
- Some of the checks generate 4 warnings that look like 100 because the file is used everywhere
- Some of the checks generate zillions of warnings.
When I made the review, for some of you I limited the files where you are to fix the warnings
Next review: first come first serve
When you think you have advanced on the work please click on the button
What will happen: I will do the best of my efforts to every Mexico morning (More or less late afternoon in India) I will review the ones that are ready for a second review.
First come first served, maybe I only find PR 3100 and 3200 as ready for review then I will review 3100 and then 3200 after.
It might happen that you get more things to do before the PR gets merged.
Careful: keeping your branch up to date
Before you start working (on any project)
Lets have the following remotes
- origin: its the clone of your fork of the project
- upstream: its the main repository of the project
Start your day with
git fetch upstream
In our case the modifications are to be done to develop.
If you see that the develop branch has changed do a rebase to your working branch:
git rebase upstream/develop
Fix conflicts, and when the rebase has finished continue working.
Merged
When it gets merged, you are very happy
If it never gets merged, don’t be sad, just make sure that you are following the reviews in the best of your abilities.
Not everything gets merged as an example here are some of my open PR on another project. On my closed PR some were merged and some didn’t make the cut.
emoji to each message in this thread that you read.
