[QC-1243]: Fix consistency in beam type names #2540
Conversation
justonedev1
requested review from
Barthelemy,
IsakovAD,
iravasen and
knopers8
as code owners
justonedev1
force-pushed
the
QC-1243
branch
2 times, most recently
from
cc0c602 to
461e1b3
Compare
justonedev1
changed the title
knopers8
left a comment
knopers8
left a comment
There was a problem hiding this comment.
Thanks, it looks good. I would only propose a minor improvement to the documentation.
doc/Advanced.md
Outdated
| It allows to have variations of the parameters depending on the run and beam types. The proper run types can be found here: [ECSDataAdapters.h](https://github.com/AliceO2Group/AliceO2/blob/dev/DataFormats/Parameters/include/DataFormatsParameters/ECSDataAdapters.h#L54). The `default` can be used | ||
| to ignore the run or the beam type. | ||
| The beam type is one of the following: `PROTON-PROTON`, `Pb-Pb`, `Pb-PROTON`, `cosmic`. | ||
| The beam type is one of the following: `pp`, `PbPb`, `pPb`, `cosmic`. |
There was a problem hiding this comment.
| The beam type is one of the following: `pp`, `PbPb`, `pPb`, `cosmic`. | |
| The beam type comes from the parameter `pdp_beam_type` set by ECS and can be one of the following: `pp`, `PbPb`, `pPb`, `pO`, `OO`, `NeNe`, `cosmic`, `technical`. | |
| See `[readout-dataflow](https://github.com/AliceO2Group/ControlWorkflows/blob/master/workflows/readout-dataflow.yaml)` to verify the possible values. |
Given that we changed the source of this, we can be more helpful in explaining where the expected values come from.
|
Actually, could you also grep the code of user modules for the affected string uses? I just saw one in the Felix' PR, so I imagine there might be more... |
|
I did grep over whole repo (hence the other reviewers for this PR than you and Barth). Reason why you see it in his code might be because he is using current master as a base branch where the changes are not present. I can wait until he merges, rebase this on new master and check again. |
|
I meant other uses, which could be already in master. I understand that his PR is not master. In any case, I'm happy that there are no other uses, then we are good to go, I believe. |
This ticket was created to address ticket QC-1243. The script is meant to be run on STG and PROD after installation of flp-suite with the changes.
However please don't merge this, I am not 100% sure that everything is working and I wanted to trigger our CI/CD tests.