8000 chore: rename `gitlab/__version__.py` -> `gitlab/_version.py` by JohnVillalovos · Pull Request #1838 · python-gitlab/python-gitlab · GitHub
[go: up one dir, main page]

Skip to content

chore: rename gitlab/__version__.py -> gitlab/_version.py #1838

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

Merged
merged 1 commit into from
Jan 15, 2022

Conversation

JohnVillalovos
Copy link
Member
@JohnVillalovos JohnVillalovos commented Jan 14, 2022

It is confusing to have a gitlab/__version__.py because we also
create a variable gitlab.__version__ which can conflict with
gitlab/__version__.py.

For example in gitlab/const.py we have to know that
gitlab.__version__ is a module and not the variable due to the
ordering of imports. But in most other usage gitlab.__version__ is a
version string.

To reduce confusion make the name of the version file
gitlab/_version.py.

8000
@codecov-commenter
Copy link
codecov-commenter commented Jan 14, 2022

Codecov Report

Merging #1838 (b981ce7) into main (cbbe7ce) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1838   +/-   ##
=======================================
  Coverage   92.26%   92.26%           
=======================================
  Files          77       77           
  Lines        4849     4849           
=======================================
  Hits         4474     4474           
  Misses        375      375           
Flag Coverage Δ
cli_func_v4 81.29% <100.00%> (ø)
py_func_v4 80.26% <100.00%> (-0.05%) ⬇️
unit 83.29% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gitlab/_version.py 100.00% <ø> (ø)
gitlab/__init__.py 100.00% <100.00%> (ø)
gitlab/const.py 100.00% <100.00%> (ø)
gitlab/mixins.py 91.50% <0.00%> (ø)
gitlab/v4/objects/files.py 100.00% <0.00%> (ø)

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/version_mv branch 2 times, most recently from a0cdb71 to d9dfb5b Compare January 14, 2022 01:55
@nejch
Copy link
Member
nejch commented Jan 15, 2022

I don't think it's that confusing as it's quite a common pattern (e.g. https://github.com/mtkennerly/poetry-dynamic-versioning/blob/4454e5082bf1fc2f4a39fa523a9b665eda5d638f/poetry_dynamic_versioning/__init__.py#L86-L89 among others).

version.py could also be confusing as we have https://docs.gitlab.com/ee/api/version.html, so maybe we could do _version.py then, to signify it's not anything to do with it?

@JohnVillalovos
Copy link
Member Author
JohnVillalovos commented Jan 15, 2022

I don't think it's that confusing as it's quite a common pattern (e.g. https://github.com/mtkennerly/poetry-dynamic-versioning/blob/4454e5082bf1fc2f4a39fa523a9b665eda5d638f/poetry_dynamic_versioning/__init__.py#L86-L89 among others).

I'm as big a fan of cargo-culting as the next person. But in our case due to the ordering of imports it is confusing. Because at some points in the code __version__ is the version string, but in other locations it is a module. Which I have found confusing when trying to work on the code as I need to understand where in the import cycle we are to know which value it is.

I did update the commit message to point out one actual situation of confusion.

version.py could also be confusing as we have https://docs.gitlab.com/ee/api/version.html, so maybe we could do _version.py then, to signify it's not anything to do with it?

Done!

It is confusing to have a `gitlab/__version__.py` because we also
create a variable `gitlab.__version__` which can conflict with
`gitlab/__version__.py`.

For example in `gitlab/const.py` we have to know that
`gitlab.__version__` is a module and not the variable due to the
ordering of imports. But in most other usage `gitlab.__version__` is a
version string.

To reduce confusion make the name of the version file
`gitlab/_version.py`.
@JohnVillalovos JohnVillalovos changed the title chore: rename gitlab/__version__.py -> gitlab/version.py chore: rename gitlab/__version__.py -> gitlab/_version.py Jan 15, 2022
@nejch
Copy link
Member
nejch commented Jan 15, 2022

Ahh ok, that makes sense. Thanks!

@nejch nejch merged commit 8af403c into main Jan 15, 2022
@nejch nejch deleted the jlvillal/version_mv branch January 15, 2022 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0