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.

Sign up for GitHub

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

Jump to bottom

3rd round of cppcheck warning fixes #5127

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

3rd round of cppcheck warning fixes #5127

wants to merge 5 commits into from

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Jan 8, 2025

Whoops, I accidently branched this off part 2, so only look at top 5 commits here...

@seanm seanm requested a review from N-Dekker January 8, 2025 18:12
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Examples Demonstration of the use of classes type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Segmentation Issues affecting the Segmentation module area:Numerics Issues affecting the Numerics module labels Jan 8, 2025
dzenanz
dzenanz reviewed Jan 8, 2025
View reviewed changes
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on a glance. I will let Niels review more carefully 😄

@jhlegarreta
Copy link
Member

ghotsflow checker seems not to detect WIP in commit ad59f8b. @bradking Any idea why? @seanm do not forget to remove it.

@seanm
Copy link
Contributor Author

seanm commented Jan 9, 2025

ghotsflow checker seems not to detect WIP in commit ad59f8b. @bradking Any idea why? @seanm do not forget to remove it.

Indeed this is not ready for merging. Need people that know that code to look at this commit... cppcheck's warnings are sensible, but the solution is not obvious to me... depends what the author meant to do...

@seanm seanm marked this pull request as draft January 9, 2025 01:12
@bradking
Copy link
Member

bradking commented Jan 9, 2025

The commit message starts in COMP: so ghostflow-director doesn't care what's after it.

@dzenanz
Copy link
Member

dzenanz commented Jan 9, 2025

With #5125 merged, it is time to rebase this and do any other preparations for review.

@seanm seanm force-pushed the cppcheck2.16.0-part3 branch from ad59f8b to 8bbe463 Compare January 9, 2025 20:10
@github-actions github-actions bot removed area:Examples Demonstration of the use of classes area:Filtering Issues affecting the Filtering module area:Segmentation Issues affecting the Segmentation module area:Numerics Issues affecting the Numerics module labels Jan 9, 2025
@seanm
Copy link
Contributor Author

seanm commented Jan 9, 2025

With #5125 merged, it is time to rebase this and do any other preparations for review.

Rebased. Now people need to look at 8bbe463 especially...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dzenanz dzenanz dzenanz left review comments

@N-Dekker N-Dekker Awaiting requested review from N-Dekker

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area:Core Issues affecting the Core module area:IO Issues affecting the IO module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seanm @jhlegarreta @bradking @dzenanz