-
Notifications
You must be signed in to change notification settings - Fork 17
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
add template file for GHA #11
Conversation
Hi Leo, @lcolladotor |
Hi Marcel @LiNk-NY, Hi Leonardo @lcolladotor, I setup GHA for new project and I came across the issue, that I need GHA to work with devel on master and with release on another branch due to changes in S4Vectors (the renaming of vertical_slot_names to parallel_slot_names). Wouldn't it make sense, to suggest the GHA to be active by default for a branch != Let me know, what you think. Thanks. Felix |
Hi Felix, @FelixErnst |
Hi Marcel, I didn't want to suggest the GHA not working 😄 . I am sorry if that's how it came across. The suggestion is mostly about the first 3 lines in the GHA template file, which don't restrict the GHA to a certain branch by default. For example, if
Vice verse if
Also leaving the on argument empty works at the beginning, but within a release cycle, this may change.
|
Hi Felix, @FelixErnst
No worries, I didn't interpret it that way. I was trying to say that it is designed to work based on the user's current Bioconductor installation. That is, if you are running Bioc-devel, the file that is generated would be for devel.
AFAICR, this is not necessary when you have a yaml file in each branch. I could be wrong though.
The code is dependent on
See point above. The normal workflow is to have the |
Ok maybe using it will tell us, how it behaves during two release cylces.
My guess is (As you said we couldn't test it yesterday), that once a package has a RELEASE branch, the GHA config will be applied as defined for all branches. At that point a new devel version is available, which is not necessarily compatible with the on before the RELEASE branch was created, In that scenario, using
Of course, but adding a GHA for the master branch only, will lead to expected behavior on the master branch and will solve future issues on RELEASE branches by not being run on those. So I would add at the limit to the master branch in GHA template by default. Otherwise users might "complain" after half a year. With the limit they might "complain" as well, but not because of an error, but because they need help to apply GHA to a different branch. But hey, this is really a minor issue. Thanks for adding the template! |
Hi Felix! @FelixErnst
Have a look at this release branch package with a GitHub Action: https://github.com/waldronlab/cBioPortalData/blob/RELEASE_3_11/.github/workflows/main.yml
There is no version mix up because the GHA will be using the appropriate Docker container or BiocManager command. As long as containers are up to date, that should not be a problem. |
Wow, this is a really fancy PR! |
Thanks Leo! I'm trying to do the package justice 😉 |
Hi! This is an older discussion on the GHA forum that was partly why I ended up having the code duplication for docker then mac & windows https://github.community/t/run-matrix-job-on-macos-and-on-ubuntu-in-container/16359. Having said that, the template could have common pieces of code and re-use the code that is duplicated. |
I've been working on this for a while, but well, I'm a stuck. I got your version to work, then started adding the features that |
At lcolladotor/testmatrix@285cfbe (aka https://github.com/lcolladotor/testmatrix/blob/285cfbe4dc277fd71e9de5ada77b2eed8dff65a1/.github/workflows/check-bioc.yml) I got the cache to work with bioc-docker and my test was successful ^^ https://github.com/lcolladotor/testmatrix/runs/1192083901?check_suite_focus=true#step:9:22 I'll merge your PR tomorrow then =) I'm still thinking about the part of setting the versions vs having the GHA workflow detect them from the git branch when you make a git push. Your approach is likely better though it involves a few more commits (every time bioc-devel changes, to update the R version) which I know is actually just once per year. I guess that also invites users to download the latest GHA workflow on Anyway, this will be a major change since it reduces nearly all the code duplication (the R cache part is duplicated right now, though that can be solved with the config matrix if the temp directory path is consistent on Windows; that's where the R user library lives; I'll check more logs tomorrow). Thanks again! |
I added a note about the change in functionality at actions/check-bioc.yml
@lcolladotor
This will create a working
check-bioc.yml
in the.github/workflows
directory based on theuser's Bioconductor version and R version.
Windows and Mac tests are not supported yet.