Skip to content

Conversation

@f3sch
Copy link
Contributor

@f3sch f3sch commented Jun 10, 2025

CI will check if this compiles.
I think it is not advisable to rely on some internal tracking data structures, we might internally do something different in the future which is not visible from the outside, rather rely on the core library.

@github-actions
Copy link

github-actions bot commented Jun 10, 2025

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

@github-actions github-actions bot changed the title PWGLF: do not include ITS internal code [PWGLF] do not include ITS internal code Jun 10, 2025
Copy link
Collaborator

@romainschotter romainschotter left a comment

Choose a reason for hiding this comment

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

Hi @f3sch ! Thanks a lot for the changes! I also took the opportunity to remove the filling with dummy values, when it is not necessary.
Could you please also repeat those changes for the converter task straevselsconverter5? You might need to update your branch first.
Thanks a lot!

@f3sch
Copy link
Contributor Author

f3sch commented Jun 10, 2025

sorry @romainschotter, I don't like this. I understand wanting to clean up stuff but these are not my changes and should go into a separate pr under your name and your responsibility. I don't want to touch code I don't have to or understand for that matter, please remove your commits from here, thanks.

@f3sch
Copy link
Contributor Author

f3sch commented Jun 10, 2025

failures in CI due to commits by @romainschotter, previously green

@alibuild
Copy link
Collaborator

Error while checking build/O2Physics/o2 for 124a775 at 2025-06-10 21:35:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/11540-slc9_x86-64/0/PWGLF/TableProducer/Strangeness/Converters/straevselsconverter4.cxx:46:29: error: 'const struct o2::soa::Table<o2::aod::Hash<1181601379>, o2::aod::Hash<2782540407>, o2::aod::Hash<2286545062> >::TableIteratorBase<o2::soa::DefaultIndexPolicy, o2::soa::Table<o2::aod::Hash<1181601379>, o2::aod::Hash<2782540407>, o2::aod::Hash<2286545062> > >' has no member named 'sumAmpFT0CInTimeRange'; did you mean 'SumAmpFT0CInTimeRange'?
ninja: build stopped: subcommand failed.

Full log here.

@romainschotter
Copy link
Collaborator

@f3sch I completely understand and respect that you would prefer to keep the PR limited to your own changes and not take on responsibility for code you are not directly involved with. That said, please keep in mind that ultimately it's Francesca's and my responsibility to review and approve the PR, so sometimes we may propose adjustments that align with broader maintenance or consistency goals.

In any case, I have removed my commits as requested.

One point I would still like to emphasize: it would be really helpful if you could propagate the changes you have made to straevselsconverter2, straevselsconverter3, and straevselsconverter4 also to the straevselsconverter5 task.
Thanks a lot for your work!

f3sch added a commit to f3sch/O2Physics that referenced this pull request Jun 11, 2025
Please consider the following formatting changes to AliceO2Group#11540
@f3sch
Copy link
Contributor Author

f3sch commented Jun 11, 2025

I don't disagree with you if it would have been within the scope of the changes proposed.

Thanks for pointing to straevselsconverter5.
Note I changed now the template type to some bogus type to unneeded avoid linkage.

@romainschotter
Copy link
Collaborator

@f3sch I understand, no worries :-)
The PR looks very good! One last comment: can you please change O2::ITStracking to O2::ReconstructionDataFormats for straevselsconverter5 in the CMakeLists.txt too?
Thanks a lot for all the work!

@romainschotter romainschotter enabled auto-merge (squash) June 11, 2025 11:03
@romainschotter romainschotter merged commit bb4736d into AliceO2Group:master Jun 11, 2025
11 of 14 checks passed
hernasab pushed a commit to hernasab/O2Physics that referenced this pull request Jun 11, 2025
prottayCMT pushed a commit to prottayCMT/O2Physics2024 that referenced this pull request Jun 12, 2025
lietava pushed a commit to lietava/O2Physics that referenced this pull request Jun 14, 2025
ddobrigk pushed a commit to ddobrigk/O2Physics that referenced this pull request Jun 14, 2025
jinhyunni pushed a commit to jinhyunni/O2Physics that referenced this pull request Jun 18, 2025
alibuild pushed a commit to alibuild/O2Physics that referenced this pull request Aug 11, 2025
jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025
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