stjudecloud / workflows

Bioinformatics workflows developed for and used on the St. Jude Cloud project.
MIT License
33 stars 10 forks source link

ci: use public Sprocket action #180

Closed adthrasher closed 3 days ago

adthrasher commented 3 days ago

Switch to the public Sprocket action instead of installing Rust and Sprocket on the runner manually.

a-frantz commented 3 days ago

Should we run this twice? Once on "check mode" and once on "lint mode"? Also, maybe should be changed upstream (after I approved it 😬 ) but --deny-warnings doesn't need to be specified on top of --deny-notes. It probably makes more sense to collapse that into one argument with 3 options (fail on errors, warnings, or notes).

adthrasher commented 3 days ago

Should we run this twice? Once on "check mode" and once on "lint mode"? Also, maybe should be changed upstream (after I approved it 😬 ) but --deny-warnings doesn't need to be specified on top of --deny-notes. It probably makes more sense to collapse that into one argument with 3 options (fail on errors, warnings, or notes).

We could certainly run it twice. For the arguments, GitHub Actions doesn't seem to support input typing at the action-level. A workflow can specify types (choice, boolean, and environment), but it doesn't appear that individual actions have that functionality.

a-frantz commented 3 days ago

Should we run this twice? Once on "check mode" and once on "lint mode"? Also, maybe should be changed upstream (after I approved it 😬 ) but --deny-warnings doesn't need to be specified on top of --deny-notes. It probably makes more sense to collapse that into one argument with 3 options (fail on errors, warnings, or notes).

We could certainly run it twice. For the arguments, GitHub Actions doesn't seem to support input typing at the action-level. A workflow can specify types (choice, boolean, and environment), but it doesn't appear that individual actions have that functionality.

Well if there's no good way to collapse deny-warnings and deny-notes I guess that's fine. It's just not the friendliest interface.

a-frantz commented 3 days ago

in sprocket check the template dir isn't being excluded properly. Once that's fixed I'll approve

adthrasher commented 3 days ago

in sprocket check the template dir isn't being excluded properly. Once that's fixed I'll approve

Yeah, I'm unclear on why that is happening. When I run the same thing locally, it picks up the excluded patterns correctly. I'm working on a change to the action to address it.