search

CambridgeCentreForProteomics / camprotR

https://cambridgecentreforproteomics.github.io/camprotR/
MIT License
4 stars 0 forks source link

Drop travis #18

Closed csdaw closed 3 years ago

csdaw commented 3 years ago

Drop Travis CI in favour of Github Actions, resolves #16.

I wasn't sure if Github Actions would work when enabled on a branch, and the answer is it won't! So I think I'll have to merge this with master to see if it actually works or not.

csdaw commented 3 years ago

Nevermind, making the pull request prompted the workflow to begin. Although the Ubuntu checks are failing as MSnbase dependencies aren't installed.

I'll try using a Bioconductor-specific Github Actions workflow instead (see biocthis).

csdaw commented 3 years ago

Almost ready to go. Only the MacOS check is failing because of a recent update to the withr package that has not been added to CRAN yet (see https://cloud.r-project.org/bin/macosx/contrib/4.0/).

Once withr is at v2.4.1 then merge this PR.

TomSmithCGAT commented 3 years ago

Thanks for this Charlotte!

csdaw commented 3 years ago

I generated .github/workflows/check-bioc.yml using biocthis and then modified it slightly. Technically biocthis is not an official Bioconductor core package. However it was the easiest way I found to implement GitHub actions CI with a package like camprotR that Imports Bioconductor packages.

csdaw commented 3 years ago

I can't really review .github/workflows/check-bioc.yml but I understand this is coming from Bioconductor. Is that correct? Aside from adding cutr, have you made other significant updates? Anything in particular in this file that needs reviewing? From my perspective, if it runs the testing OK, all fine with me 👍

No other significant changes! I just want to get this into master and then I'll pull it into the {ts}_test_test_test branch and start writing more tests.

TomSmithCGAT commented 3 years ago

Can't you merge this branch into {ts}_test_test_test directly in that case. Or am I missing something?

csdaw commented 3 years ago

Yes you can. Good point I hadn't thought of that 😅 . I think I can edit the PR to change the base branch?

TomSmithCGAT commented 3 years ago

Great. So just waiting on withr update then?

csdaw commented 3 years ago

Nope that's already solved otherwise the MacOS check wouldn't pass. I'll merge now 😁.