Feat: Additional Github Workflow Actions #37
Conversation
[resolving merge conflict broke CMakeLists, hopefully this patches it]
…r to keep track of breaking changes
henry-wallace-phys
changed the title
| VALIDATE_PYTHON: true # Enable Python linting | ||
| VALIDATE_ALL_CODEBASE: true # Lint the entire codebase | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # GitHub token for permissions | ||
| # DISABLE_LINTERS: "CPP" # Disable linting for other languages |
There was a problem hiding this comment.
Can we really get away with the CPP linter enabled? I guess you've dealt with this by including the continue-on-error true? I think it's more useful to disable the CPP linter and require errors on YAML to be fixed
There was a problem hiding this comment.
Like for example, you can see a YAML error here: https://github.com/DUNE/MaCh3_DUNE/actions/runs/13117147270/job/36594095657#step:4:939
To me, it's more important that we catch these YAML errors and have the bot fail
| find_package(MaCh3 1.3.5 EXACT QUIET) | ||
|
|
||
| # KS: Here we try to find tag matching tutorial version. If we can't find one then use develop | ||
| # This will allow to grab tutorial for tagged MaCh3 version without a need of manually changing version |
There was a problem hiding this comment.
Is this really an upgrade?
It's now requiring two strings to be updated anytime we update core version - I don't think there's any new feature here?
There was a problem hiding this comment.
I guess it's related to the flexible core update - but I don't know if that's really a good thing to support - need more discussion
| @@ -17,6 +19,10 @@ RUN git checkout ${MACH3_DUNE_VERSION} | |||
| RUN mkdir -p ${MACH3_DUNE_INSTALL_DIR} | |||
| WORKDIR ${MACH3_DUNE_INSTALL_DIR} | |||
|
|
|||
| RUN cmake ../ | |||
| RUN if [ -n "$MACH3_CORE_VERSION" ]; then \ | |||
There was a problem hiding this comment.
I think this is also potentially going to lead to issues - typically, the DUNE code is only supportable against a particular tag of core. It's not guaranteed that a snapshot of the DUNE code can be used against any core version. We also want to make sure that for reproducibility, the tag of core in the CMakeLists.txt is the one that is actually used
| @@ -0,0 +1,248 @@ | |||
| @article{gelman1996posterior, | |||
There was a problem hiding this comment.
Again, I don't know if we actually need this
Pull request description:
Ports more workflows from T2K/Core MaCh3
Changes or fixes:
Landmine Check
https://github.com/tylermurry/github-pr-landmine/tree/master
Lets you check if a suggested code modification in a PR will just break the CI without needing to download the entire codebase!
Notes:
Branched off of doxygen PR so requires that pulled in first. As these actions are supplementary I've kept the PRs separate