search

ImperialCollegeLondon / python-template

A template for Python projects using cookiecutter
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

Update CI files to be more efficient #24

Closed AdrianDAlessandro closed 1 month ago

AdrianDAlessandro commented 2 months ago

Use @cc-a's minimal workflow? https://github.com/cc-a/minimal_ci_workflow

alexdewar commented 1 month ago

I already said this in the meeting earlier, but for the sake of posterity: we need to test both poetry and pip-tools because they're different code paths. And I think we should at least test Linux and Windows because this cookiecutter template involves a script running and I can imagine it breaking on one or the other... We could probably drop macOS though... What does everyone think?

@dalonsoa @cc-a

dalonsoa commented 1 month ago

I think it makes sens to test in everything. Granted it is not the fastest thing, but as @alexdewar is the kind of thing that we want to be working in multiple scenarios. As there are no releases we cannot just offload the full matrix testing to just before a release.

AdrianDAlessandro commented 1 month ago

Fair enough. I'll close this issue

AdrianDAlessandro commented 1 month ago

I've just realised that I mean we should change the CI that will be used in the downstream repos that are built using this tool. Not the CI used in this repo. So everything in https://github.com/ImperialCollegeLondon/python-template/blob/main/%7B%7B%20cookiecutter.project_slug%20%7D%7D/.github/workflows/ci.yml

alexdewar commented 1 month ago

@AdrianDAlessandro You mean using @cc-a's minimal workflow for the template instead of what we have now?

I feel like we should keep the Windows runners, because these really do behave differently sometimes (when it happens I usually get failures from path-related issues -- ugh), but the macOS runners are probably less important, because if it works on Linux it'll probably work on macOS. That said, I suppose with the new ARM Macs, you might have packaging-related problems, so you might want to check for that.

One thing we could do instead is add extra prompts to the template, so users can opt in/out of any of the runners. We could even make having CI workflows optional altogether.

What does everyone think? @dalonsoa @cc-a

AdrianDAlessandro commented 1 month ago

@AdrianDAlessandro You mean using @cc-a's minimal workflow for the template instead of what we have now?

Yes, exactly.

I feel like we should keep the Windows runners, because these really do behave differently sometimes (when it happens I usually get failures from path-related issues -- ugh), but the macOS runners are probably less important, because if it works on Linux it'll probably work on macOS. That said, I suppose with the new ARM Macs, you might have packaging-related problems, so you might want to check for that.

The template doesn't exclude being able to run windows, in fact it includes it in the release_test workflows (although they aren't actually run on release but pushes to main). Also, I feel like path issues really shouldn't be an issue with python if we're using pathlib for all paths, but I know it's not just that.

Whether we use that exact template is up for discussion. It assumes a dual-branch setup (dev and main), which I'm not the biggest fan of. But the main point is if we've just had this discussion around limiting our CI use in an intelligent way, our python template should reflect those takeaways.

One thing we could do instead is add extra prompts to the template, so users can opt in/out of any of the runners. We could even make having CI workflows optional altogether.

I think extra prompts are great! Especially for things like needing windows-specific compatibility or specific python versions, or for selecting which of the different CI options we have (pre-commit, tests, builds, etc).

cc-a commented 1 month ago

Agree it would be nice to have something more intelligently configurable. Would need to think carefully about the different CI options the user could choose between though. Certainly the exact workflow in https://github.com/cc-a/minimal_ci_workflow makes assumptions about development practices that may not be relevant for all projects and confusing to the average research user.

alexdewar commented 1 month ago

Agree with both of you. I def want to use what we learned, but I don't want to assume users have a separate develop and main branch because that could make things confusing.

Maybe for now, we just make what's there a bit more configurable by letting users opt in to the different runners...?

cc-a commented 1 month ago

That sounds sensible.