[PWGCF] add 2 and 4 particle correlation and fix errors#16619
Conversation
|
O2 linter results: ❌ 1 errors, |
|
Error while checking build/O2Physics/o2 for a7a4270 at 2026-06-11 12:50: Full log here. |
|
Error while checking build/O2Physics/staging for a7a4270 at 2026-06-11 12:53: Full log here. |
|
@pykuan You are supposed to test your changes before you make a PR. Please follow the contribution guidelines. |
Sorry, the clang-format in local didn't work properly. It caused even more errors than I expected... I'll try to fix O2 linter error and the code itself first. |
clang-format cannot cause errors if you use it as explained in the documentation and it has definitely nothing to do with your broken compilation. |
|
How exactly are you using clang-format? |
I used "clang-format -i someFile" but it somehow breaks lines that are supposed to be in a single line, and causes errors. I saw there are many configurables in clang-format but didn't know which to switch off to satisfy the automatic checks. |
There is no such instruction in the documentation to use |
From me. I was using that regularly for local formatting before committing code to the O2physics repository, starting in 2023. I also used it a few months ago when making my last commit, and there were no problems or complaints. I do not remember from where I got it any longer in 2023, but for more than 2 years (at least), there were no problems with that. Can you please clarify to us why we cannot use suddenly |
|
Hi @abilandz
You can use whatever you want but:
|
If my memory does not betray me, when I was getting first formatting errors on my very first commits in O2Physics, I was getting automatically generated commits from aliBuild with the title "Please consider the following formatting changes", or something like that. I think that after inspecting the online log, I realized they were generated with And I completely eliminated from my workflow in O2Physics these unnecessary automatically generated commits from aliBuild related to the formatting. Btw, the flag "-i" is irrelevant for this discussion - it merely updates the files in-place after the formatting (in the same fashion as
This is not a fair summary. We have used `clang-format' successfully for more than two years, basically in all my commits in the past 2+ years. @pykuan also used it successfully in her commits in the past few weeks. So something very recently has changed in the workflow related to the formatting. Could you please clarify what has changed? Do we use some other tool now instead of
I do not consider executing Are you suggesting now that we should not attempt to format ourselves the code locally, and just rely on these automated tools you are mentioning? But if I can format in the very same way my code locally, that should only ease the burden on those automated tools... Therefore, I do not fully understand your point here. |
abilandz
left a comment
There was a problem hiding this comment.
I see most of the formatting problems are fixed, therefore I approve this one. Please address the remaining 1 error from O2linter in the next PR.
It is still the case. See above.
I don't know what you find "not fair" but the facts are simple. If your local formatting worked, there would be no formatting PR created. It was created, therefore your local formatting does not work.
No, nothing has changed very recently. The last change of the clang-format version was in March.
No. The options have not changed as far as I can remember.
It's trivial and wrong because you are missing the
Yes. It's written in the documentation.
If you prefer extra work and want to do it locally by hand, do it at least properly and do not confuse students with wrong instructions. Otherwise, there is a configured and documented pre-commit hook for this. |
It is still the case. See above.
I don't know what you find "not fair" but if your local formatting worked, there would be no formatting PR created. It was created, therefore your local formatting does not work.
No, nothing has changed very recently. The last change of the clang-format version was in March.
No. The options have not changed as far as I can remember.
It's trivial and also wrong because you are missing the
Yes. It's written in the documentation.
If you prefer extra work and want to do it locally by hand, do it at least properly and do not confuse students with wrong instructions. Otherwise, there is a configured and documented pre-commit hook for this. |
Our local formatting was working for 2+ years. Without any change on our side, it stopped working suddenly. This looks like a change was introduced to the central workflow related to the formatting, which is not backward-compatible.
You could have replied that in your very first post here. But your reply now is nevertheless incomplete - looking at the documentation of Can you please disclose what we use for argument
Please can you re-think that part of the documentation, and add the following line: Before making a commit for files file1 file2 ... fileN, a developer is encouraged to execute locally: This would completely eliminate the need for aliBuild to automatically generate trivial commits related to formatting. I guess we all agree that we should not clog our O2 repository with unnecessary commits. If the local formatting failed for whatever reason, only then aliBuild can generate the commit related to formatting, but with the instruction at the end on how a developer can run the very same formatting tool locally, so that such a trivial and unnecessary formatting commit does NOT have to be generated by aliBuild itself.
It's not extra work, and it's a really trivial one-liner - I am sure you understand that. Doing formatting "by hand" would imply we are manually making an intervention in the source code locally to fix the formatting - nobody normal would ever attempt to do that. And, btw, we get all clang suite installed with the O2 package in any case...
|
No description provided.