Closed LiNk-NY closed 3 years ago
Hm. I guess that I could make the template use conditional code. Like a biocthis::use_bioc_github_action(dynamic = TRUE)
which would use the current version, and if dynamic = FALSE
then it would use the proposed workflow.
Hi Leo, @lcolladotor I've taken a stab at this in PR #11. It would use the user's Bioconductor version (whether release or devel) to fill in the GHA template.
I'm just going through that PR, it's really fancy!! Thanks Marcel!!
Thank you Marcel for the great PR! After a bunch of tests at https://github.com/lcolladotor/testmatrix and a few commits on biocthis
, I think that we can close this issue.
Note that I had to revert some of the code from your PR https://github.com/lcolladotor/biocthis/pull/11: the sysreqs
part is one of them. This commit shows most of the changes https://github.com/lcolladotor/biocthis/commit/66a6232632f88f605921622d467bf863c5c871e2. Something else I used was remotes::install_cran("rcmdcheck")
which doesn't update if it detects that the latest version is there already.
I also added arguments to biocthis::use_bioc_github_action()
to control setting the main environment variables (pkgdown
, covr
, RUnit
, testhat
) on the GHA workflow. I added to the R package some of the previous GHA code for finding the R version that matched a given BioC version so .GHARversion()
looks quite different.
I'll close this issue, but we might want to revise some details here and there. Anyway, this version works now ^^
I added you as a ctb
^^ Hopefully that's ok =)
Thanks for looking it over Leo! @lcolladotor
I agree with all the changes required to make it work on non-Linux platforms. I didn't have a chance to test it :sweat_smile:
I don't think the .GHARversion
has to be that complicated given the use cases as detailed in previous responses. Perhaps they weren't clear so I will provide an actual example.
Suppose I am using Bioconductor devel and I would like to add / update the GitHub Action for my repository. I go into my session with the corresponding setup which has this Bioconductor and R version:
> BiocManager::version()
[1] '3.12'
# AND
> getRversion()
[1] '4.0.2'
Note. My package checkout should always correspond to the branch that I am in:
~/bioc/biocthis (master) $ git branch
* master
For the Bioconductor release scenario, a typical setup would be like this and
using a hypothetical release version of biocthis
:
> BiocManager::version()
[1] '3.11'
# AND
> getRversion()
[1] '4.0.2'
~/bioc/biocthis (RELEASE_3_11) $ git branch
* RELEASE_3_11
master
So it would be configured properly if the developer is using the appropriate setup :wink:.
When a Bioc release is out, BiocManager
should always be the source of the correct R version and Bioc version, when
BiocManager::valid()
is TRUE
.
Great idea to have optional modules such as:
pkgdown = TRUE,
testthat = TRUE,
covr = testthat,
RUnit = FALSE
IMO, I'd make pkgdown
and testthat FALSE
by default. I think the aim is to get the testing done and have the user add any additional features at their leisure.
I'd also restrict pkgdown
to only the release
branch since devel
changes often and documentation isn't really published for devel branches.
Thanks!
So it would be configured properly if the developer is using the appropriate setup 😉.
Hehe true true. Though, I've also written packages say on Bioc-release and used Travis (and now GHA) on Bioc-devel to make sure it's all good. So ehem, this more complicated .GHARversion()
helps me... sometimes be lazy ^^'. I agree with you that for the scenario where a developer is working in the correct setup, that there's no need to have the complicated .GHARversion()
.
About pkgdown
and testthat
, I use them in all my packages (ok, some have very low coverage values, ooops!). I see your points though, and maybe one way is to use getOption()
so I can configure my .Rprofile
to match my preferences. I have found pkgdown
to be really helpful to check the rendered documentation of my package. It makes it easier to read and to identify typos and things like that. Creating it automatically really helps speed things up also. I also really like how pkgdown
provides links, which makes the documentation much easier to navigate, hence why I have pkgdown
websites for all my BioC packages.
Setting up a pkgdown release and devel would need more work, unlike what we have at BioC. Because I'm making changes that users will get to see in the next release version, I want to proof-read them and all that, that's why my docs use the devel branch and not the release one. Though well, after using biocthis::use_bioc_github_action()
, a user can re-configure the GHA workflow. Hm... I guess that I could add a pkgdown_branch
argument to the template function though.
I added the getOption()
code and a pkgdown_covr_branch
argument which by default is master
.
Hi Leo, @lcolladotor
I think we can make the
bioc_check.yaml
file more simple since we are invoking the function from R to generate the file. We can use the R instance to dictate the version of the docker images.For example, if the developer runs
biocthis::use_bioc_github_action()
with a Bioc-devel set-up, the generatedyaml
file should usebioconductor/bioconductor_docker:devel
. This will remove the need to create a step to verify the appropriate R version.https://github.com/lcolladotor/biocthis/blob/master/actions/check-bioc.yml
Also, I think this file should be under some folder like
inst/templates/check-bioc.yml
. The code can then useglue
orwhiskers
or whatever package people use nowadays to replace the Bioconductor version in theyaml
file after doingsystem.file(..., package = "biocthis")
(Related to #8)
Best, Marcel