Skip to content

[Common] setting up irSource automatically using the CollisionHelper #13282

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.

Sign up for GitHub

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

Jump to bottom
Merged
ddobrigk merged 7 commits into from
Oct 28, 2025
Merged

[Common] setting up irSource automatically using the CollisionHelper #13282

ddobrigk merged 7 commits into from
Oct 28, 2025

Conversation

@amaringarcia
Copy link
Contributor

@amaringarcia amaringarcia commented Oct 8, 2025

No description provided.

@github-actions github-actions bot added the common label Oct 8, 2025
@github-actions github-actions bot changed the title setting up irSource automatically using the CollisionHelper [Common] setting up irSource automatically using the CollisionHelper Oct 8, 2025
@github-actions
Copy link

github-actions bot commented Oct 8, 2025 •
edited
Loading

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

@alibuild alibuild mentioned this pull request Oct 8, 2025
Closed
vkucera
vkucera reviewed Oct 8, 2025
View reviewed changes
@alibuild alibuild mentioned this pull request Oct 9, 2025
Closed
vkucera
vkucera reviewed Oct 9, 2025
View reviewed changes
@alibuild
Copy link
Collaborator

alibuild commented Oct 9, 2025

Error while checking build/O2Physics/o2 for b266ce4 at 2025-10-09 18:38:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/13282-slc9_x86-64/0/Common/Tools/PID/pidTPCModule.h:715:16: error: variable 'bc' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/13282-slc9_x86-64/0/Common/Tools/PID/pidTPCModule.h:715:16: error: variable 'bc' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/13282-slc9_x86-64/0/Common/Tools/PID/pidTPCModule.h:715:16: error: variable 'bc' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/13282-slc9_x86-64/0/Common/Tools/PID/pidTPCModule.h:715:16: error: variable 'bc' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/13282-slc9_x86-64/0/Common/Tools/PID/pidTPCModule.h:715:16: error: variable 'bc' set but not used [-Werror=unused-but-set-variable]
ninja: build stopped: subcommand failed.

Full log here.

@amaringarcia
Copy link
Contributor Author

amaringarcia commented Oct 10, 2025

I would be very grateful if the pull request can also be approved

@vkucera
Copy link
Collaborator

vkucera commented Oct 20, 2025

Please fix the reported bugs.

@alibuild alibuild mentioned this pull request Oct 22, 2025
Closed
vkucera
vkucera reviewed Oct 22, 2025
View reviewed changes
@amaringarcia
Copy link
Contributor Author

amaringarcia commented Oct 22, 2025

@vkucera please let me know if the code is ok now. The left over warnings are only "lowerCamelCase "....
I believe I have removed the bugs.
Thanks for the valuable comments/suggestions.
Ana

@vkucera
Copy link
Collaborator

vkucera commented Oct 22, 2025

@vkucera please let me know if the code is ok now. The left over warnings are only "lowerCamelCase ".... I believe I have removed the bugs. Thanks for the valuable comments/suggestions. Ana

Hi @amaringarcia , thanks for the fixes. Please note that the k prefix should only be used for special constants, i.e. sizes of C-style arrays.

@vkucera
Copy link
Collaborator

vkucera commented Oct 23, 2025

Thanks for addressing my comments.

@amaringarcia
Copy link
Contributor Author

amaringarcia commented Oct 24, 2025

@ddobrigk could you have a look at approve at your earliest convenience....

@amaringarcia
Copy link
Contributor Author

amaringarcia commented Oct 28, 2025

Hi @ktf @ddobrigk @iarsene @jgrosseo can any of you allow the merging of the pull request? The changes suggested by Vit were implemented. Thanks in advance.

@ddobrigk
Copy link
Collaborator

ddobrigk commented Oct 28, 2025

Hi @ktf @ddobrigk @iarsene @jgrosseo can any of you allow the merging of the pull request? The changes suggested by Vit were implemented. Thanks in advance.

Hi @amaringarcia sorry for the delay, this looks okay: approving.

@ddobrigk ddobrigk merged commit 47c97e8 into AliceO2Group:master Oct 28, 2025
12 of 13 checks passed
ThePhDane pushed a commit to ThePhDane/O2Physics that referenced this pull request Nov 3, 2025
@amaringarcia @ThePhDane
[Common] setting up irSource automatically using the CollisionHelper (A…
f292788 
…liceO2Group#13282)

Co-authored-by: Ana Marin <marin@lxbk0907.gsi.de>
Co-authored-by: Ana Marin <marin@lxbk0905.gsi.de>
lmattei01 pushed a commit to lmattei01/O2Physics that referenced this pull request Dec 5, 2025
@amaringarcia
[Common] setting up irSource automatically using the CollisionHelper (A…
3f82303 
…liceO2Group#13282)

Co-authored-by: Ana Marin <marin@lxbk0907.gsi.de>
Co-authored-by: Ana Marin <marin@lxbk0905.gsi.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkucera vkucera vkucera left review comments

@ddobrigk ddobrigk ddobrigk approved these changes

@alibuild alibuild Awaiting requested review from alibuild alibuild is a code owner

@jgrosseo jgrosseo Awaiting requested review from jgrosseo jgrosseo is a code owner

@iarsene iarsene Awaiting requested review from iarsene iarsene is a code owner

@ktf ktf Awaiting requested review from ktf ktf is a code owner

Assignees

No one assigned

Labels

common

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amaringarcia @alibuild @vkucera @ddobrigk