iox-#889 Fix clang-tidy violations for iceoryx_hoofs package

[r7vme]

1. Fix all clang-tidy-10 violations in iceoryx_hoofs package.
2. Add CI job run-clang-tidy that runs check (w/o failing as agreed with @dkroenke) for iceoryx_hoofs

To test locally:

1. Make sure to install clang-tidy-10
2. ./tools/ci/run-clang-tidy.sh

Pre-Review Checklist for the PR Author

1. Code follows the coding style of CONTRIBUTING.md
2. Tests follow the best practice for testing
3. Changelog updated in the unreleased section including API breaking changes
4. Branch follows the naming format (iox-#123-this-is-a-branch)
5. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#123 commit text)
   • Commit messages are signed (git commit -s)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
6. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
7. Relevant issues are linked
8. Add sensible notes for the reviewer
9. All checks have passed (except task-list-completed)
10. Assign PR to reviewer

Notes for Reviewer

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• Unit tests have been written for new behavior
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Partially addresses Add CI job that runs clang-tidy #889

[codecov]

Codecov Report

Merging #895 (ab305a3) into master (8e2edeb) will increase coverage by 0.43%.
The diff coverage is 70.93%.

@@            Coverage Diff             @@
##           master     #895      +/-   ##
==========================================
+ Coverage   75.71%   76.14%   +0.43%     
==========================================
  Files         335      335              
  Lines       12222    12267      +45     
  Branches     2038     2051      +13     
==========================================
+ Hits         9254     9341      +87     
+ Misses       2178     2134      -44     
- Partials      790      792       +2     

Flag	Coverage Δ
unittests	75.07% <62.79%> (+0.43%)	⬆️
unittests_timing	29.98% <51.16%> (+0.39%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...iceoryx_hoofs/internal/file_reader/file_reader.hpp	100.00% <ø> (ø)
...ceoryx_hoofs/include/iceoryx_hoofs/log/logging.hpp	100.00% <ø> (ø)
...hoofs/log/logging_free_function_building_block.hpp	93.33% <0.00%> (ø)
...ryx_hoofs/include/iceoryx_hoofs/log/logmanager.hpp	100.00% <ø> (ø)
...include/iceoryx_hoofs/posix_wrapper/posix_call.hpp	100.00% <ø> (ø)
.../include/iceoryx_hoofs/posix_wrapper/semaphore.hpp	100.00% <ø> (ø)
...ude/iceoryx_hoofs/posix_wrapper/signal_handler.hpp	0.00% <ø> (ø)
iceoryx_hoofs/platform/linux/source/file.cpp	100.00% <ø> (ø)
iceoryx_hoofs/platform/linux/source/fnctl.cpp	100.00% <ø> (ø)
iceoryx_hoofs/platform/linux/source/grp.cpp	100.00% <ø> (ø)
... and 53 more

[r7vme]

@elBoberido @elfenpiff @dkroenke please take a look this PR fixes clang-tidy violations in iceoryx_hoofs

[r7vme]

@elBoberido @elfenpiff @dkroenke @mossmaurice

Sorry for a huge PR. If you have time please take a look. And I welcome any suggestions.

There is also a new CI job that runs clang-tidy check for iceoryx_hoofs package.

[elBoberido]

Just skimmed over some files and didn't do a thorough review. Is this really a PR with 12858 insertions and 365 removals? I think there is something fishy. Do we maybe have a line ending change?

It would also have been better to create multiple PRs for this since it has quite some changes where small details have a big impact ebeded in an ocean of trivial changes.

[elBoberido]

Please don't do it this way. With this you still have to do creationMove(*this, std::move(rhs)) which caused the linter warnings or creationMove(*this, std::move(*static_cast<Creation<Foo>*>(&rhs))) which we want to prevent with this method.

Please rewrite this to

void creationMove(Creation<Class, Error>& rhs)
{
    *this = std::move(rhs);
}

On the caller site this would be just a creationMove(rhs);.

This might cause some warnings though. In this case, please use a macro.

[elBoberido]

please revert and check that all the code changes doing similar stuff is also reverted

[elBoberido]

Are these linter attributes still necessary? If yes, we need to find a solution which makes them obsolete. It's kind of embarrassing to have these in our code base.

[dkroenke]

@elfenpiff @elBoberido @r7vme I see that this does not make much sense here with this huge PR, too much open topics and clang-tidy exceptions in the code. I propose that this PR goes into draft state until the reviewers have agreed on migration strategy.

[elBoberido]

Maybe some parts can be reverted and and only the non controversial stuff be merged first. The naming for example makes up most of the PR and I think we can all agree on this. Stuff like with the move of the creation pattern should be left out and be done in a follow up.

This PR should then also be squashed before merging else there are huge intermediate changes which mess up history. There are for sure not 12858 additions in this PR. Something is fishy with that.

[dkroenke]

Maybe some parts can be reverted and and only the non controversial stuff be merged first. The naming for example makes up most of the PR and I think we can all agree on this. Stuff like with the move of the creation pattern should be left out and be done in a follow up.

This PR should then also be squashed before merging else there are huge intermediate changes which mess up history. There are for sure not 12858 additions in this PR. Something is fishy with that.

@elBoberido
The reason for the huge diff was a generated file from a unittest that accidently was committed. I modified the test and added the file to gitignore.
The controversial parts in the move assignment operators are for now excluded from clang-tidy and needs to be addressed later, once an agreement is found.
@elfenpiff This is an agenda point for the next developer meetup.

Maybe that has more chances now.

The controversial parts are for now excluded and tracked in the ticket