-
Notifications
You must be signed in to change notification settings - Fork 256
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
Particle type added for special reconstruction for triton and He3 #1224
Conversation
da952c6: approval required: 1 of @qgp (Jochen Klein), @shahor02 (Ruben Shahoyan), @sawenzel (Sandro Christian Wenzel), @pzhristov (Peter Hristov), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @chiarazampolli (Chiara Zampolli), @davidrohr (David Rohr), @ktf (Giulio Eulisse) Comment with |
@@ -36,6 +36,7 @@ | |||
#include "AliESDtrack.h" | |||
#include "AliMCEvent.h" | |||
#include "AliTOFPIDParams.h" | |||
#include "AliPID.h" |
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.
Is this include needed? It was not there before, and AliPID was used.
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.
If the include was coming from other dependencies, its explicit inclusion will not do any harm, so, it is ok
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.
Yes, sure, I would have avoided it if not needed, but I totally agree. I was curious to know why @mcolocci added it.
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.
Ciao @chiarazampolli. About the AliPID dependence could be indeed avoided. I added at the beginning because we were seeing an error during the rebuild and at the first glance we thought they were related to it. Then, when I managed to rebuild fixing the error who was somewhere else…, I simply forgot to remove it. Thanks for checking!
+1 |
da952c6: approved: will be automatically merged on successful tests |
Yes, I also wrote that it is anyway harmless (but I asked Manuel, for completeness). What I wanted you to check was the rest :-) Chiara |
@chiarazampolli I've looked the code also, looks fine. The class version does not need to be incremented if only static data member is added, but it also will not harm. |
Error while checking build/AliRoot/release for da952c6 at 2020-09-17 20:39:
Full log here. |
Thanks Ruben! Error expected and unrelated. Chiara |
Error while checking build/AliRoot/macos for da952c6 at 2020-09-20 08:51:
Full log here. |
Hello @ktf , Will this PR go through the tests automatically after the change by @ihrivnac in alidist (alisw/alidist#2488)? Cheers, Chiara |
No description provided.