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

Fix Clang warnings #2201

Merged
merged 3 commits into from
Nov 8, 2021
Merged

Fix Clang warnings #2201

merged 3 commits into from
Nov 8, 2021

Conversation

heplesser
Copy link
Contributor

This PR fixes a large number of warnings emitted by Apple Clang. Almost all of these warnings were due to explicit template class instantiations for Datum types, which could be removed by moving minimal methods from _impl.h files to .h files. This allowed removal of several _impl.h and .cc files.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Nov 3, 2021
@heplesser heplesser added this to To do in Installation via automation Nov 3, 2021
jougs
jougs approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

Thanks for this cleanup. I did not follow each move individually, but trust the fact that the CI is happy.

Installation automation moved this from To do to Review Nov 5, 2021
hakonsbm
hakonsbm suggested changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@hakonsbm hakonsbm 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. I just have one very minor formatting suggestion.

nestkernel/parameter.cpp Outdated Show resolved Hide resolved
@heplesser @hakonsbm
Removed superfluous blank line.
b925efe 
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
hakonsbm
hakonsbm approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Thanks!

@hakonsbm hakonsbm merged commit 2d0bad0 into nest:master Nov 8, 2021
Installation automation moved this from Review to Done Nov 8, 2021
@heplesser heplesser deleted the fix-clang-warnings branch April 20, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jougs jougs jougs approved these changes

@hakonsbm hakonsbm hakonsbm approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Installation
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@heplesser @jougs @hakonsbm