Skip to content

Tidy up #98

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

Merged
merged 5 commits into from
Mar 26, 2025
Merged

Tidy up #98

merged 5 commits into from
Mar 26, 2025

Conversation

Ozaq
Copy link
Contributor

@Ozaq Ozaq commented Mar 14, 2025

This PR enables a few more tidy checks and addresses them. Additionally it adds a way for git blame to ignore changed from our big reformatting.

Removed unneeded if
There is no need to null check a pointer before deleting because 'delete
ptr' where prt = nullptr is a NO-OP.

Add .git-blame-ignore-revs
'git blame' can be configured to ignore specific commits such as
reformatting the codebase with clang format.

We track the commits we want to be ignored in .git-blame-ignore-revs.

You can use git blame from the cli with:
git blame --ignore-revs-file .git-blame-ignore-revs

Or you configure your git config to always use .git-blame-ignore-revs by
setting:
git config blame.ignoreRevsFile .git-blame-ignore-revs

Or extending your gitconfig with:

[blame]
ignoreRevsFile = .git-blame-ignore-revs

Note that .git-blame-ignore-revs is recognized by GitHub.

Introduce tidy bugprone-use-after-move
Move- constructor / assignment operator can be defaulted in this case.
Silencing the use-after-move warning. In this specific case there was no
bug in the existing code because we moved only the base class part.

Add aditional tidy checks

  • bugprone-assignment-in-if-condition
  • bugprone-bool-pointer-implicit-conversion
  • bugprone-unused-return-value,

Added new tidy-check and fixed warning
Added bugprone-casting-through-void

@Ozaq Ozaq requested a review from simondsmart March 14, 2025 21:17
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.89%. Comparing base (9e8f9d5) to head (6153927).

Files with missing lines Patch % Lines
src/fdb5/remote/server/ServerConnection.cc 0.00% 3 Missing ⚠️
src/fdb5/io/FieldHandle.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #98      +/-   ##
===========================================
- Coverage    63.90%   63.89%   -0.01%     
===========================================
  Files          281      281              
  Lines        15809    15804       -5     
  Branches      1644     1642       -2     
===========================================
- Hits         10102    10098       -4     
+ Misses        5707     5706       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ozaq Ozaq force-pushed the tidy-up branch 3 times, most recently from 9b6295d to c0825d9 Compare March 18, 2025 10:34
Ozaq added 5 commits March 26, 2025 10:46
There is no need to null check a pointer before deleting because 'delete
ptr' where prt = nullptr is a NO-OP.
'git blame' can be configured to ignore specific commits such as
reformatting the codebase with clang format.

We track the commits we want to be ignored in .git-blame-ignore-revs.

You can use git blame from the cli with:
    git blame --ignore-revs-file .git-blame-ignore-revs <FILE>

Or you configure your git config to always use .git-blame-ignore-revs by
setting:
    git config blame.ignoreRevsFile .git-blame-ignore-revs

Or extending your gitconfig with:

[blame]
    ignoreRevsFile = .git-blame-ignore-revs

Note that .git-blame-ignore-revs is recognized by GitHub.
Move- constructor / assignment operator can be defaulted in this case.
Silencing the use-after-move warning. In this specific case there was no
bug in the existing code because we moved only the base class part.
- bugprone-assignment-in-if-condition
- bugprone-bool-pointer-implicit-conversion
- bugprone-unused-return-value,
Added bugprone-casting-through-void
@Ozaq
Copy link
Contributor Author

Ozaq commented Mar 26, 2025

@simondsmart simondsmart merged commit cede261 into develop Mar 26, 2025
120 of 146 checks passed
@simondsmart simondsmart deleted the tidy-up branch March 26, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants