search

lcolladotor / biocthis

Automate package and project setup for Bioconductor packages
https://lcolladotor.github.io/biocthis/
50 stars 17 forks source link

[BUG] Unexpected?? differences in Bioc builder report and github actions #28

Closed rmflight closed 2 years ago

rmflight commented 2 years ago

Using the GH actions setup (and some customization based on packages, etc) supplied by {biocthis} and running, I expected mostly similar output as what is observed on the Bioconductor build reports for an already accepted package. However, that isn't what I've experienced. There are warnings and notes in the GH actions that I don't see on the Bioconductor build report. I'm not sure how much this is to be expected (hence the "unexpected??" in the subject of this issue).

Context

I wanted to suggest some changes to a maintainer / author of an already accepted Bioconductor package, SDAMS. And I wanted to have GH actions for it so I would know if I'm introducing anything weird that might cause the Bioconductor builds to error out before they accept the changes. {biocthis} GH actions setup seemed like the easiest, quickest way to get that going.

However, the current build reports on Bioconductor release and devel show no issues with this package.

My github actions, even though they are now completing, show some issues with the package, specifically the Rnw vignette.

Any insight you can provide on these differences would be awesome to help me understand whats going on.

lcolladotor commented 2 years ago

Hi,

So, in this case, you should compare the bioc-release vs GHA since that's what you have at https://github.com/MoseleyBioinformaticsLab/SDAMS/blob/master/.github/workflows/check-bioc.yml#L55

At https://www.bioconductor.org/checkResults/release/bioc-LATEST/SDAMS/ this is what I see:

image

From there, you can see the GitHub sha of the version that was tested. To make it apples to apples, it should be the same one as the commit ID on GHA.

Assuming it is, next let's look at the output of R CMD check:

image

So there things match. Next, GHA also runs BiocCheck which Bioconductor doesn't do on the BBS (Bioconductor Build System), only in the SBP (Single Package Builder) which is used for new submissions.

Bioconductor only really uses BiocCheck for new submissions to help check packages. Once packages are part of Bioconductor, I guess that they made the decision not to run BiocCheck to reduce computational costs. Also, they likely assumed that a package author wouldn't break a package later on. Or at least in no big way. It's also true that BiocCheck has evolved over time and some Bioconductor packages were accepted before these some new tests were added to BiocCheck.

Overall, I see no fault in biocthis and if you want, you can turn off BiocCheck completely by deleting https://github.com/MoseleyBioinformaticsLab/SDAMS/blob/master/.github/workflows/check-bioc.yml#L269-L279 or you could manually turn off some pieces of BiocCheck https://github.com/MoseleyBioinformaticsLab/SDAMS/blob/master/.github/workflows/check-bioc.yml#L273-L278 like I do at say https://github.com/lcolladotor/derfinder/blob/master/.github/workflows/check-bioc.yml#L260-L266.

Best, Leo

rmflight commented 2 years ago

Great! Thanks Leo. That really does help to understand any differences observed.

I didn't think there was any fault in biocthis or BiocCheck, everything there seems like sane things to test. I guess it was more me misunderstanding what exactly was being run by the default GHA biocthis workflow vs the Bioconductor builds.