search

UCL-ARC / python-tooling

Python package template for new research software projects
http://github-pages.arc.ucl.ac.uk/python-tooling/
MIT License
43 stars 2 forks source link

Renaming `github_username` to `github_owner` + generating `__repo_name` & `__repo_url` #409

Closed matt-graham closed 3 weeks ago

matt-graham commented 5 months ago

Though its not exposed to user now we have longer prompts rather than using variable names directly, I think github_username is a bit of a misnomer given that it can be either a user or organization name, hence I think github_owner would be a better choice.

We also currently manually construct the GitHub repository URL in lots of different bits of the template and also the qualified repository name in a few places. We can use double underscore prefixed (rendered) private variables in the cookiecutter config to generate the repository URL and qualified name once and then reuse elsewhere to avoid the repetition. This would also make it simpler to later switch to supporting alternative repository hosting options such as GitLab in future (which is the rational for naming the variables __repo_name and __repo_url rather than __github_repo_name and __github_repo_url).

dstansby commented 5 months ago

I'm 👍 , but reluctant to approve before we have some regression tests (https://github.com/UCL-ARC/python-tooling/issues/329) since there's a lot of changes here that could be susceptible to human error (e.g., missing or adding "/" characters from URLs)

samcunliffe commented 5 months ago

I'm 👍 , but reluctant to approve before we have some regression tests (https://github.com/UCL-ARC/python-tooling/issues/329) since there's a lot of changes here that could be susceptible to human error (e.g., missing or adding "/" characters from URLs)

Isn't it enough the tests we currently have, all pass? We do do a cookiecut operation in CI, right? Which is working...

https://github.com/UCL-ARC/python-tooling/blob/2477eee282b58b8083e96013cae35843e10cf6b9/tests/test_package_gen.py#L7

matt-graham commented 3 weeks ago

Tests were previously failing since #432 was merged in as the passed configs now need to use github_owner rather than github_username - now fixed and have also updated tutorial to make more consistent with new github_owner variable name.

samcunliffe commented 3 weeks ago

Let's make v1.0.0 of the template when this is in.