Skip to content

[PWGCF] adding new analysis for multiharmonic correlations#16457

Merged
abilandz merged 3 commits into
AliceO2Group:masterfrom
pengchon:master
May 30, 2026
Merged

[PWGCF] adding new analysis for multiharmonic correlations#16457
abilandz merged 3 commits into
AliceO2Group:masterfrom
pengchon:master

Conversation

@pengchon
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2026

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

@github-actions github-actions Bot changed the title adding new analysis for multiharmonic correlations [PWGCF] adding new analysis for multiharmonic correlations May 29, 2026
@vkucera
Copy link
Copy Markdown
Collaborator

vkucera commented May 29, 2026

Duplicate of #16403

@vkucera vkucera marked this as a duplicate of #16403 May 29, 2026
@vkucera vkucera closed this May 29, 2026
@abilandz abilandz reopened this May 30, 2026
@abilandz
Copy link
Copy Markdown
Collaborator

Duplicate of #16403

Dear @vkucera, that one was closed because the student didn't have a fork in sync with the master, and after making a commit, there were merging conflicts. Instead of wrestling with them, we decided to close that PR, sync the fork, and start over with a new and clean PR.

Your comments on 16403 are taken into account in this PR.

Thanks for your understanding!

@abilandz
Copy link
Copy Markdown
Collaborator

@pengchon : The compilation on MacOS-arm is failing due to

/Volumes/build/alice-ci-workdir/o2physics/sw/SOURCES/O2Physics/16457/0/PWGCF/MultiparticleCorrelations/Tasks/multiharmonicCorrelations.cxx:440:11: error: variable 'eta' set but not used [-Werror,-Wunused-but-set-variable] 440 | float eta = 0; | ^ 1 error generated.

Please can you push a new commit with this fix? This one is mandatory.

Also, see if you can fix the remaining 2 errors for O2linter quickly, otherwise address them in the next PR.

Thanks!

@abilandz abilandz merged commit bfecdc7 into AliceO2Group:master May 30, 2026
13 checks passed
@vkucera
Copy link
Copy Markdown
Collaborator

vkucera commented May 30, 2026

Duplicate of #16403

Dear @vkucera, that one was closed because the student didn't have a fork in sync with the master, and after making a commit, there were merging conflicts. Instead of wrestling with them, we decided to close that PR, sync the fork, and start over with a new and clean PR.

Your comments on 16403 are taken into account in this PR.

Thanks for your understanding!

Dear @abilandz , I understand the problem you were facing, but closing the PR is completely unrelated to it and unnecessary, especially when the new PR is opened from the same branch as the closed one. The student could have just updated his branch and pushed without closing the initial PR. Please avoid the duplication in the future.

@vkucera
Copy link
Copy Markdown
Collaborator

vkucera commented May 30, 2026

Also I still don't see a single delete statement in the new code.

@abilandz
Copy link
Copy Markdown
Collaborator

Duplicate of #16403

Dear @vkucera, that one was closed because the student didn't have a fork in sync with the master, and after making a commit, there were merging conflicts. Instead of wrestling with them, we decided to close that PR, sync the fork, and start over with a new and clean PR.
Your comments on 16403 are taken into account in this PR.
Thanks for your understanding!

Dear @abilandz , I understand the problem you were facing, but closing the PR is completely unrelated to it and unnecessary, especially when the new PR is opened from the same branch as the closed one. The student could have just updated his branch and pushed without closing the initial PR. Please avoid the duplication in the future.

Dear @vkucera, if nothing else, the new PR is clean, with only 3 necessary commits, since it doesn't include any commits that merely resolve merge conflicts. We could have reverted all commits in the closed PR and continued pushing new commits through that PR, but for us it was easier to do it this way - as you can see, everything worked smoothly with the new PR. And it should be easier now to trace back in the future which particular commit introduced new features (or bugs), if we ever need to find that out.

But based on your feedback, we will try to avoid closing any future PR - as always, thanks for your feedback!

@abilandz
Copy link
Copy Markdown
Collaborator

Also I still don't see a single delete statement in the new code.

Apologies for that, this was skipped most likely accidentally - it's an easy fix and will be done in the next PR.

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