Skip to content

Fixup cpp check#15

Merged
nmellado merged 5 commits intomainfrom
fixupCppCheck
Mar 25, 2024
Merged

Fixup cpp check#15
nmellado merged 5 commits intomainfrom
fixupCppCheck

Conversation

@hiergaut
Copy link
Copy Markdown
Contributor

  • [cmake] Update cppcheck target
  • need cppcheck passed without error

@hiergaut hiergaut requested a review from nmellado March 21, 2024 09:35
@hiergaut
Copy link
Copy Markdown
Contributor Author

hiergaut commented Mar 21, 2024

I am splitting this branch to separate old server implement removing from this feature branch, please do not merge this branch until process done.

@hiergaut
Copy link
Copy Markdown
Contributor Author

Splitted, waiting now for #16 merged.

@hiergaut hiergaut force-pushed the fixupCppCheck branch 3 times, most recently from bc846e2 to bd6138e Compare March 22, 2024 09:22
@hiergaut

This comment was marked as outdated.

@hiergaut
Copy link
Copy Markdown
Contributor Author

hiergaut commented Mar 22, 2024

Not ready, some tests failed

@hiergaut
Copy link
Copy Markdown
Contributor Author

tests passed, @nmellado this PR is ready to merge

@nmellado
Copy link
Copy Markdown
Contributor

I've cleaned the history.

Question: why removing mqtt tests in this PR ?

@hiergaut
Copy link
Copy Markdown
Contributor Author

this cmake line list(FILTER test_srcs EXCLUDE REGEX ".*Mqtt\\.cpp$") did the job.
It removed Mqtt test if Mqtt library not found
The lines I commented are useless now because I use one directory per src module
We can rename the commit "Disable Mqtt tests" -> "Removed useless mqtt cmake removal"

@hiergaut
Copy link
Copy Markdown
Contributor Author

Can I do the commit name change ?

Copy link
Copy Markdown
Contributor

@nmellado nmellado left a comment

Choose a reason for hiding this comment

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

this cmake line list(FILTER test_srcs EXCLUDE REGEX ".*Mqtt\\.cpp$") did the job. It removed Mqtt test if Mqtt library not found The lines I commented are useless now because I use one directory per src module We can rename the commit "Disable Mqtt tests" -> "Removed useless mqtt cmake removal"

Ok, makes sense.
Please :

  • reword commit message as "[cmake] Remove dead code"
  • in cmake, remove lines instead of commenting out
  • in CHANGELOG, add entry [cmake] Remove dead code (#15)

@hiergaut
Copy link
Copy Markdown
Contributor Author

@nmellado this PR is ready to merge

@nmellado nmellado merged commit dfb86e4 into main Mar 25, 2024
@nmellado nmellado deleted the fixupCppCheck branch March 25, 2024 14:30
@hiergaut hiergaut mentioned this pull request Apr 22, 2024
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.

2 participants