Skip to content

Conversation

@vkucera
Copy link
Collaborator

@vkucera vkucera commented Dec 8, 2025

  • Remove unused cxx files.
  • Fix cxx files wrongly used as headers.
    • Rename to h.
    • Add header guards.
    • Fix fully qualified names.
    • Move using directives to source files.
  • Fix includes.
    • Fix paths to RecoDecay.
    • Fix missing C++ system headers.
    • Fix include style.

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

O2 linter results: ❌ 425 errors, ⚠️ 423 warnings, 🔕 11 disabled

@github-actions github-actions bot changed the title Fix wrong headers. Remove unused files. [PWGJE] Fix wrong headers. Remove unused files. Dec 8, 2025
@vkucera vkucera changed the title [PWGJE] Fix wrong headers. Remove unused files. [PWGJE] Fix wrong headers. Remove unused files. Fix includes. Dec 8, 2025
vkucera pushed a commit to vkucera/O2Physics that referenced this pull request Dec 8, 2025
@vkucera vkucera marked this pull request as ready for review December 8, 2025 22:09
vkucera pushed a commit to vkucera/O2Physics that referenced this pull request Dec 13, 2025
@vkucera
Copy link
Collaborator Author

vkucera commented Dec 18, 2025

@nzardosh I have validated that o2-analysis-je-jet-substructure-d0 produces the same histogram content before and after this PR.

Configurable<std::vector<double>> trackingEfficiency{"trackingEfficiency", {1.0, 1.0}, "tracking efficiency array applied to jet finding if applyTrackingEfficiency is true"};

Service<o2::framework::O2DatabasePDG> pdg;
o2::framework::Configurable<float> zCut{"zCut", 0.1, "soft drop z cut"};
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the benefit of not including these common o2 namespaces in the .h files? it seems to me that every single o2 task needs these namespaces so it just seems it makes the headers more complex to read if we remove them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using directives are forbidden in the global scope of headers for many important reasons.

@nzardosh
Copy link
Collaborator

thanks for checking vit! I am happy with the headers being moved but I am less sure that making the namespaces explicit in the headers is practical. particularly the ones that are used in every single task.

@nzardosh nzardosh merged commit 420b113 into AliceO2Group:master Dec 19, 2025
11 of 13 checks passed
@vkucera vkucera deleted the je-headers branch December 19, 2025 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants