Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable clang-tidy checks and fix warnings in code #1196

Closed
85 tasks done
elfenpiff opened this issue Mar 2, 2022 · 2 comments · Fixed by #1455, #1454, #1457, #1462 or #1459
Closed
85 tasks done

Enable clang-tidy checks and fix warnings in code #1196

elfenpiff opened this issue Mar 2, 2022 · 2 comments · Fixed by #1455, #1454, #1457, #1462 or #1459
Assignees
Labels
bugfix Solves a bug clang-tidy Warning of a clang-tidy rule globex technical debt unclean code and design flaws
Milestone

Comments

@elfenpiff
Copy link
Contributor

elfenpiff commented Mar 2, 2022

Brief feature description

At the moment a lot of clang-tidy checks in our root .clang-tidy file are disabled to ensure that our codebase is at least warning free with a minimal subset of clang-tidy checks.

We should enable the disabled checks one by one and solve the generated warnings in the same step. Some of the warnings require a larger refactoring. If this happens please create another issue and add a todo link in this issue.

ToDo

  1. Remove the compiler warning suppression from all tests, see CMakeLists.txt and the entry set(TEST_CXX_FLAGS PRIVATE ${ICEORYX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS} -Wno-pedantic -Wno-conversion) and fix all warnings.
  2. Create a .clang-tidy file in every test disables warning which originate from the googletest fixtures itself. For instance TEST_F and TYPED_TEST have a rule of 5 warning.

Hints

The .clang-tidy file may have warnings which do not fit for us. If this is the case then please disable the warning and write a justification why this is rule is not appropriate. Example:

* rule: readability-identifier-length
  justification: In some functions single letter variables make sense, this is covered by the review process
  example:
     bad:
       int sum(int summand1, int summand2); // here we would like to have single letter variable arguments
     good:
       int sum(int a, int b); // failure since a and b have less than three letters

CI checks

Files

When a file is stated, it means the header, inline file, cpp file and also the tests are refined.

iceoryx_hoofs/cxx

iceoryx_hoofs/units

iceoryx_hoofs/concurrent

iceoryx_hoofs/rp

posix_wrapper

TODOs after merge of the integration branch

@elfenpiff elfenpiff added bugfix Solves a bug technical debt unclean code and design flaws labels Mar 2, 2022
@elfenpiff elfenpiff mentioned this issue Mar 2, 2022
3 tasks
@dkroenke dkroenke added the globex label Jun 8, 2022
@elBoberido elBoberido added this to the Low prio milestone Jun 9, 2022
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
…ts and correct first part of cxx tests

Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit that referenced this issue Jul 6, 2022
…in-binding-c-tests

iox-#1196 Fix compiler warnings in binding c tests
FerdinandSpitzschnueffler added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
…make STACK_SIZE in test uint64_t again

Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
FerdinandSpitzschnueffler added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
FerdinandSpitzschnueffler added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
FerdinandSpitzschnueffler added a commit to ApexAI/iceoryx that referenced this issue Jul 6, 2022
Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Aug 23, 2022
…fully usable for robust mutex, address review findings

Signed-off-by: Christian Eltzschig <me@elchris.org>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Aug 24, 2022
Signed-off-by: Christian Eltzschig <me@elchris.org>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Aug 24, 2022
Signed-off-by: Christian Eltzschig <me@elchris.org>
elfenpiff added a commit that referenced this issue Aug 24, 2022
…r-concurrent

Iox #1196 activate clang tidy for concurrent
@elBoberido elBoberido modified the milestones: Low prio, High prio Sep 6, 2022
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Sep 12, 2022
…fully usable for robust mutex, address review findings

Signed-off-by: Christian Eltzschig <me@elchris.org>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Sep 21, 2022
…fully usable for robust mutex, address review findings

Signed-off-by: Christian Eltzschig <me@elchris.org>
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Sep 22, 2022
…fully usable for robust mutex, address review findings

Signed-off-by: Christian Eltzschig <me@elchris.org>
elBoberido added a commit to ApexAI/iceoryx that referenced this issue Nov 1, 2022
@elBoberido elBoberido reopened this Nov 1, 2022
elBoberido added a commit that referenced this issue Nov 1, 2022
…tic-accessed-through-instance-rule

iox-#1196 Disable readability-static-accessed-through-instance rule
mossmaurice pushed a commit that referenced this issue Jan 17, 2023
Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment