Skip to content
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

Refactoring TRestDetectorReadout classes. #86

Merged
merged 21 commits into from
Jun 5, 2023
Merged

Conversation

jgalan
Copy link
Member

@jgalan jgalan commented May 31, 2023

jgalan Large: 3331

Cherry-picking from PR #83 to find out the causes of pipeline failure

  • Adding readout validation that previously was in framework

@jgalan
Copy link
Member Author

jgalan commented Jun 2, 2023

Any idea why alphaTrack Geant4 example would be failing in this PR? @juanangp @lobis

@lobis
Copy link
Member

lobis commented Jun 2, 2023

Any idea why alphaTrack Geant4 example would be failing in this PR? @juanangp @lobis

Ni idea sorry, I'll try to figure it out

@jgalan
Copy link
Member Author

jgalan commented Jun 2, 2023

Any idea why alphaTrack Geant4 example would be failing in this PR? @juanangp @lobis

Ni idea sorry, I'll try to figure it out

Ok, I see now that the example is using a readout, so it is normal. It needs to be regenerated.

@jgalan
Copy link
Member Author

jgalan commented Jun 5, 2023

HI @juanangp I have added a new validation job to validation.yml. But I don't see it in the jobs summary. Perhaps it will take effect only once this PR has been merged?

@jgalan jgalan requested a review from juanangp June 5, 2023 07:33
@juanangp
Copy link
Member

juanangp commented Jun 5, 2023

HI @juanangp I have added a new validation job to validation.yml. But I don't see it in the jobs summary. Perhaps it will take effect only once this PR has been merged?

You can change the pipeline in master to check if the pipeline suceed, this line https://github.com/rest-for-physics/framework/blob/2aae3c4346bc3914785b883438f278fc0668e358/.github/workflows/validation.yml#L645 to

    uses: rest-for-physics/detectorlib/.github/workflows/validation.yml@jgalan-refactoring

This requires to make a new branch in framework and after is merged do not remove the jgalan-refactoring branch and later on you should revert it to:

    uses: rest-for-physics/detectorlib/.github/workflows/validation.yml@master

This is the only way that you can check if the pipeline is going to suceed, otherwise you can check after is merged, but it will not be tested in advance. We should implement submodule validation in framework in order to avoid this work around.

@jgalan jgalan marked this pull request as ready for review June 5, 2023 08:00
@jgalan jgalan merged commit 811424d into master Jun 5, 2023
@jgalan jgalan deleted the jgalan-refactoring branch June 5, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants