AlexsLemonade / OpenScPCA-analysis

An open, collaborative project to analyze data from the Single-cell Pediatric Cancer Atlas (ScPCA) Portal
Other
5 stars 14 forks source link

Update code styling workflow #728

Closed sjspielman closed 1 month ago

sjspielman commented 1 month ago

Closes #708

This (draft!) PR updates the code styling workflow to run on only mature modules.

sjspielman commented 1 month ago

In practice, that approach might not exclude using a matrix, but we might have to get a bit fancy passing the list of modules from a file to the matrix definition. Pretty sure it is possible though with multiple jobs where only the second job uses a matrix strategy and then using matrix: include: to pass the list of modules to the second job. Syntax might be a bit tricky, but I think it is possible.

I think this sort of situation is about what you're describing, no? https://github.com/AlexsLemonade/AdminItems/blob/d3feaff26e9b803195b23ee25e2d5c617f3e5f10/.github/workflows/file-annual-creds-rotations.yml#L55-L81

sjspielman commented 1 month ago

Just a note that src: was good for getting ruff to run in the right directories.

jashapiro commented 1 month ago

In practice, that approach might not exclude using a matrix, but we might have to get a bit fancy passing the list of modules from a file to the matrix definition. Pretty sure it is possible though with multiple jobs where only the second job uses a matrix strategy and then using matrix: include: to pass the list of modules to the second job. Syntax might be a bit tricky, but I think it is possible.

I think this sort of situation is about what you're describing, no? https://github.com/AlexsLemonade/AdminItems/blob/d3feaff26e9b803195b23ee25e2d5c617f3e5f10/.github/workflows/file-annual-creds-rotations.yml#L55-L81

Yes, though I would probably just store the list as JSON to avoid the ugly awk

sjspielman commented 1 month ago

Well this is interesting... https://github.com/AlexsLemonade/OpenScPCA-analysis/actions/runs/10618217531/job/29432900775#step:7:61

jashapiro commented 1 month ago

Well this is interesting... https://github.com/AlexsLemonade/OpenScPCA-analysis/actions/runs/10618217531/job/29432900775#step:7:61

80% sure this is because it is running in a fork.

sjspielman commented 1 month ago

I think given the CI checks running in https://github.com/AlexsLemonade/OpenScPCA-analysis/pull/728/commits/8a550aa9a832f2e9545e3c502cfb54452c5d661d this is probably ready for review. Hopefully the PR wasn't properly filed here for because of the fork situation, since I still don't have other ideas..!

sjspielman commented 1 month ago

I'm also not sure where we should put the mature modules file; where you have it is probably fine for now, but I am not sure it should be buried in a hidden directory.

I too am not sure about this, and similarly I'm not sure about calling this "mature modules," either, because modules are at different levels of maturity. This list is specifically "what's ready to be styled," so I now think we want a file name that more specifically reflects that too.

sjspielman commented 1 month ago

This should be ready for another look! Let me know what you think about the mature modules list, which I've renamed for now (also do we want ewings in there yet?).

Note that this passed checks here (no PRs filed, but matrix jobs were run): https://github.com/AlexsLemonade/OpenScPCA-analysis/pull/728/commits/0e51c5a991a0f709d7cedd93377106d2ae2b2921