Skip to content

Conversation

@victor-gonzalez
Copy link
Collaborator

No description provided.

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

O2 linter results: ❌ 62 errors, ⚠️ 73 warnings, 🔕 0 disabled

@victor-gonzalez
Copy link
Collaborator Author

@vkucera Just wanted to move the included .cxx file to .h as you suggested
linter errors and warning will be fixed in subsequent steps
MegaLinter error is harder to fix because the file is included twice, once per task, in one of the source files so the #ifndef header guard will prevent proper compilation

@vkucera
Copy link
Collaborator

vkucera commented Dec 8, 2025

Hi @victor-gonzalez , thanks for the fix. Concerning the header guards, the whole idea of including a file in the middle of a source file to paste some code verbatim is wrong. If one needs to paste code, it should be done with a preprocessor macro which can be defined in the header which will be only included once at the top of the source. But even that is a bad idea and should be done better with properly defined C++ objects.

@victor-gonzalez
Copy link
Collaborator Author

Hi @victor-gonzalez , thanks for the fix. Concerning the header guards, the whole idea of including a file in the middle of a source file to paste some code verbatim is wrong. If one needs to paste code, it should be done with a preprocessor macro which can be defined in the header which will be only included once at the top of the source. But even that is a bad idea and should be done better with properly defined C++ objects.

I agree but that will take some time. For the time being, I will merge this one and address the full issue in the coming weeks.
Thanks!

@vkucera
Copy link
Collaborator

vkucera commented Dec 9, 2025

Hi @victor-gonzalez , thanks for the fix. Concerning the header guards, the whole idea of including a file in the middle of a source file to paste some code verbatim is wrong. If one needs to paste code, it should be done with a preprocessor macro which can be defined in the header which will be only included once at the top of the source. But even that is a bad idea and should be done better with properly defined C++ objects.

I agree but that will take some time. For the time being, I will merge this one and address the full issue in the coming weeks. Thanks!

Very good, thanks @victor-gonzalez !

@victor-gonzalez victor-gonzalez enabled auto-merge (squash) December 9, 2025 12:07
Copy link
Collaborator

@alibuild alibuild left a comment

Choose a reason for hiding this comment

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

Auto-approving on behalf of @victor-gonzalez.

@victor-gonzalez victor-gonzalez merged commit d7545a8 into AliceO2Group:master Dec 9, 2025
10 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants