Skip to content
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

Overall impression #21

Closed
lcolladotor opened this issue Nov 24, 2022 · 2 comments
Closed

Overall impression #21

lcolladotor opened this issue Nov 24, 2022 · 2 comments

Comments

@lcolladotor
Copy link

Hi Brian @bschilder,

I've just finished going through the docs & code. Thanks for pinging me about rworkflows. Overall, it's a very interesting project and I'm glad to see that biocthis was helpful in creating rworkflows, although as you noted at #3, they have diverged a bit over time. Though well, biocthis will definitely benefit from solutions you come up with when some of the GHA dependencies change / break. While I'll try to keep an eye open here and there for changes at rworkflows, I'll appreciate pings at biocthis too =)

I see that rworkflows is primarily aimed at getting people to use GHA without having to deal with what's going on to make it work. Like, you (not the user) would have to update the actions at rworkflows if some dependencies break (as we've both seen can happen with changes to say r-lib/actions, and many others).

I see that https://github.com/neurogenomics/rworkflows/blob/master/R/use_workflow.R has lots of options, though well, eventually, it's hard to list all the options available in rcmdcheck::rcmdcheck() as well as BiocCheck::BiocCheck(). For example, https://github.com/LieberInstitute/spatialLIBD/blob/e2f179bb7a3bfef30b319e84dc7702e3ce99aa67/.github/workflows/check-bioc.yml#L265.

rworkflows does suffer a bit from code duplication across different yml files you have, which well, means that you'll have to be careful when updating it. For example, maybe someone might send you a PR for one of the yml files and you'll need to double check that this change is applied elsewhere too. Given that you need different yml files, I don't think there is a way to get around this code duplication, but maybe you know one.

Overall, I like what you've done here =) Kudos to you!

Best,
Leo

cc'ing @grimbough @LiNk-NY who might be interested in rworkflows too and might have other things to say / share

@lcolladotor
Copy link
Author

I'll make a few other small issues with specific small comments that you should feel free to ignore

@bschilder
Copy link
Collaborator

bschilder commented Dec 9, 2022

Thank you so much for the detailed feedback @lcolladotor ! This is all extremely helpful. I'll get started on addressing these by making separate Issues for each.

Some initial thoughts:

I've just finished going through the docs & code. Thanks for pinging me about rworkflows. Overall, it's a very interesting project and I'm glad to see that biocthis was helpful in creating rworkflows, although as you noted at #3, they have diverged a bit over time. Though well, biocthis will definitely benefit from solutions you come up with when some of the GHA dependencies change / break. While I'll try to keep an eye open here and there for changes at rworkflows, I'll appreciate pings at biocthis too =)

Absolutely, will be great to coordinate these and learn from one another!

I see that rworkflows is primarily aimed at getting people to use GHA without having to deal with what's going on to make it work. Like, you (not the user) would have to update the actions at rworkflows if some dependencies break (as we've both seen can happen with changes to say r-lib/actions, and many others).

Indeed, this is definitely I'm anticipating as well. But I also think this is where version control comes in handy. Tags like "@v1" (as opposed to "@master") should in theory be pretty stable.

I see that https://github.com/neurogenomics/rworkflows/blob/master/R/use_workflow.R has lots of options, though well, eventually, it's hard to list all the options available in rcmdcheck::rcmdcheck() as well as BiocCheck::BiocCheck(). For example, https://github.com/LieberInstitute/spatialLIBD/blob/e2f179bb7a3bfef30b319e84dc7702e3ce99aa67/.github/workflows/check-bioc.yml#L265.

I think this extra degree of control would be nice for both checking functions. Will look into this further.
Issue documented here #24

rworkflows does suffer a bit from code duplication across different yml files you have, which well, means that you'll have to be careful when updating it. For example, maybe someone might send you a PR for one of the yml files and you'll need to double check that this change is applied elsewhere too. Given that you need different yml files, I don't think there is a way to get around this code duplication, but maybe you know one.

You're right, and this occurs in several areas:

  • subaction: I tried segmenting the rworkflows action into smaller subactions (here Break Actions report into steps #4), namely so I could get each group of steps to display as a separate "Jobs". i think these visual partitions make it much easier to navigate the GHA logs in the web browser. Eventually I aim to replace the merged action with an action that calls each of these subactions, so there won't be redundancy at that point.
  • rworkflows_static: This is meant to be analogous to biocthis::use_bioc_github_action(). But rworkflows_static is indeed a slightly reformatted version of rworkflows action. I think I can write a function to instead programmatically convert the action into a static workflow, rather than keeping a separately copy that might diverge by accident. Documented here: Reduce redundancy between "rworkflows" (action) vs. "rworkflows_static" (workflow) #29

Overall, I like what you've done here =) Kudos to you!

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants