-
Notifications
You must be signed in to change notification settings - Fork 614
[PWGLF] do not include ITS internal code #11540
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
Conversation
|
O2 linter results: ❌ 20 errors, |
romainschotter
left a comment
There was a problem hiding this 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!
|
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. |
|
failures in CI due to commits by @romainschotter, previously green |
|
Error while checking build/O2Physics/o2 for 124a775 at 2025-06-10 21:35: Full log here. |
|
@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 |
Please consider the following formatting changes to AliceO2Group#11540
|
I don't disagree with you if it would have been within the scope of the changes proposed. Thanks for pointing to straevselsconverter5. |
|
@f3sch I understand, no worries :-) |
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.