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

Bump FairLogger to v2.1.0 and InfoLogger to v2.8.2 #5763

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ehellbar
Copy link
Contributor

Introducing a "critical" log message severity, required to distinguish critical errors for shifter operation.

@ehellbar ehellbar requested a review from a team as a code owner February 10, 2025 15:06
@ktf
Copy link
Member

ktf commented Feb 10, 2025

It looks like there is a genuine incompatibility between InfoLogger and the new FairLogger.

@ehellbar
Copy link
Contributor Author

ok, I will check if it is only these two string_views or if there is more

@ktf
Copy link
Member

ktf commented Feb 10, 2025

Keep also in mind that c_str() is guaranteed to be 0 terminated, string_view is not.

@ehellbar
Copy link
Contributor Author

waiting for AliceO2Group/InfoLogger#95 to also bump InfoLogger here

@ehellbar ehellbar requested a review from a team as a code owner February 19, 2025 16:31
@ehellbar ehellbar changed the title Bump FairLogger to v2.1.0 Bump FairLogger to v2.1.0 and InfoLogger to v2.8.1 Feb 19, 2025
@vascobarroso vascobarroso requested a review from sy-c February 19, 2025 17:07
@sy-c
Copy link
Contributor

sy-c commented Feb 20, 2025

My bad, I messed up with the include file during the test... Will fix shortly

Copy link
Contributor

@sy-c sy-c left a comment

Choose a reason for hiding this comment

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

I fixed infologger and pushed v2.8.2

The include issue escaped my attention as I did the release integration tests using InfoLogger instead of libInfoLogger (which is the one included by the O2 components).

@ehellbar ehellbar changed the title Bump FairLogger to v2.1.0 and InfoLogger to v2.8.1 Bump FairLogger to v2.1.0 and InfoLogger to v2.8.2 Feb 20, 2025
@ehellbar
Copy link
Contributor Author

ehellbar commented Feb 20, 2025

the compiler on macOS throws a warning when using enums in switch statements (in this case fair::Severity) and not handling all of the enumerators, which is the case here (missing the case for the new critical severity introduced with this bump of FairLogger). For PR checks, we use -Werror so we get an error here. Locally on my mac, the latest O2@dev with this alidist compiles successfully, throwing only the warning.

The O2 PR to fix this is already open but needs the bump of FairLogger first: AliceO2Group/AliceO2#13957

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.

3 participants