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.

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

Fix a few warnings when compiling #187

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Fix a few warnings when compiling #187

merged 3 commits into from
Jun 20, 2024

Conversation

jmcarcell
Copy link
Contributor

BEGINRELEASENOTES

ENDRELEASENOTES

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Is this currently warning free? Then we could consider adding -Werror to CI to keep it that way? But I suppose that would need to go into the key4hep build action, right?

@jmcarcell
Copy link
Contributor Author

I don't get any warnings with this change. Adding -Werror won't work for k4EDM4hep2LcioConv so I can't add it globally yet but I'm not opposed to have it or at least try it for some time. It may be annoying when changing compiler or version and finding that a warning appears though.

@tmadlener
Copy link
Contributor

Maybe it could be an option of the action? Then it would be easier to toggle on a per project basis. Maybe we need to take the discussion to the correct repo. I will merge this now.

@tmadlener tmadlener merged commit aa8b0cb into main Jun 20, 2024
8 of 11 checks passed
@tmadlener tmadlener deleted the warnings branch June 20, 2024 07:11
@jmcarcell
Copy link
Contributor Author

jmcarcell commented Jun 20, 2024

Tagging @Zehvogel since he has complained about message verbosity; this change will modify the cases when the Gaudi message level is set to NIL, VERBOSE, FATAL, ALWAYS, NUM_LEVELS (see https://gitlab.cern.ch/gaudi/Gaudi/-/blob/master/GaudiKernel/include/GaudiKernel/IMessageSvc.h#L25), since these were not in the switch statement. At the very least m_level is set by default to NIL: https://gitlab.cern.ch/gaudi/Gaudi/-/blob/master/install/include/GaudiKernel/CommonMessaging.h#L130. I think this may change the default when the verbosity is unset (and defaults to NIL?) since it's ERROR (https://github.com/key4hep/k4MarlinWrapper/blob/main/k4MarlinWrapper/k4MarlinWrapper/MarlinProcessorWrapper.h#L68) and after this it will be MESSAGE.

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