Closed lilyminium closed 2 years ago
Hello @lilyminium! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
{{cookiecutter.repo_name}}/.github/actions/get-stable-python-version/get_python_version.py
:Line 14:80: E501 line too long (80 > 79 characters) Line 105:80: E501 line too long (106 > 79 characters)
{{cookiecutter.repo_name}}/{{cookiecutter.package_name}}/tests/analysis/test_{{cookiecutter.template_analysis_class.lower()}}.py
](https://github.com/MDAnalysis/cookiecutter-mdakit/blob/88ffc4d58a99e761987e51d4cbba015818763702/{{cookiecutter.repo_name}}/{{cookiecutter.package_name}}/tests/analysis/test_{{cookiecutter.template_analysis_class.lower()}}.py):[Line 4:80](https://github.com/MDAnalysis/cookiecutter-mdakit/blob/88ffc4d58a99e761987e51d4cbba015818763702/{{cookiecutter.repo_name}}/{{cookiecutter.package_name}}/tests/analysis/test_{{cookiecutter.template_analysis_class.lower()}}.py#L4): E501 line too long (140 > 79 characters)
@orbeckst @IAlibay @fiona-naughton your reviews and opinions on the items in the first comment would be highly appreciated, but I do realise that this is a fairly large PR. If one of you is happy with the behaviour I think it adds, and with the examples at https://github.com/MDAnalysis/cookiecutter-mdakit/tree/examples and https://github.com/MDAnalysis/mdakit-cookie branches, then I'll count that as good enough :)
I don't know if we have a quota for uploading artifacts. I set the retention time to 3 days (down from the default 90) just in case.
Yes, AFAIK there's a ~500 GB~ 500 MB quota on artifacts (I saw it when I set up the artifact uploading for the whitepaper). Reducing storage duration to 3d is a good idea.
I am in favor of putting re-useable actions into separate repos. MIT license is also totally fine with me.
I don't know if we have a quota for uploading artifacts. I set the retention time to 3 days (down from the default 90) just in case.
Yes, AFAIK there's a ~500 GB~ 500 MB quota on artifacts (I saw it when I set up the artifact uploading for the whitepaper). Reducing storage duration to 3d is a good idea.
We actually burn through this real quick as an organisation, especially when we do releases (wheels for everything is a good 200 MB being used for a few days). I'd suggest going for the minimum artifact retention period that is useful here (even down to a single day unless you expect to need to inspect / debug artifact contents a lot)
Product Storage Minutes (per month) GitHub Free for organizations 500 MB 2,000
Maybe storage is actually free for public repositories and the limits only apply to private repos. I am not 100% sure
GitHub Actions usage is free for both public repositories and self-hosted runners. For private repositories, each GitHub account receives a certain amount of free minutes and storage, depending on the product used with the account.
In any case, keeping retention times short is good stewardship of resources.
Thanks for looking this over, @IAlibay and @orbeckst! I made #27 as an issue so I can merge this CI now. I'll also keep an eye on the storage quota if the retention time becomes an issue. :)
Fixes #23 Fixes #16
This PR redoes CI, which now:
examples/
branch of this oneDespite ostensibly addressing fairly low-priority issues, this fixes CI so that it actually runs -- turns out the cookie tests weren't actually working as in the
main
branch. Therefore it's necessary.Decisions that could do with discussion:
get-stable-python-version
is reasonably re-usable, if we want to store it somewhere in the MDAnalysis/ repo and just pull from that. It's licensed under MIT and afaik I wrote it from scratch -- are we ok with that?