Skip to content

refactor: explicitly import gitlab.const values into top-level namespace #1694

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 3 commits into from
Dec 1, 2021

Conversation

JohnVillalovos
Copy link
Member

We are planning on adding enumerated constants into gitlab/const.py,
but if we do that than they will end up being added to the top-level
gitlab namespace. We really want to get users to start using
gitlab.const. to access the constant values in the future.

Explicitly add the current values defined in gitlab.const into the
top-level namespace and stop using the previous
'from gitlab.const import *' method.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/const_explicit branch 2 times, most recently from fd04824 to ba3b38a Compare November 17, 2021 00:44
@JohnVillalovos JohnVillalovos changed the title Explcitly import gitlab.const values into top-level namespace refactor: explicitly import gitlab.const values into top-level namespace Nov 17, 2021
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/const_explicit branch from ba3b38a to cf92904 Compare November 17, 2021 00:48
@codecov-commenter
Copy link

codecov-commenter commented Nov 17, 2021

Codecov Report

Merging #1694 (b3b0b5f) into main (09a973e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1694      +/-   ##
==========================================
+ Coverage   92.01%   92.02%   +0.01%     
==========================================
  Files          75       75              
  Lines        4670     4676       +6     
==========================================
+ Hits         4297     4303       +6     
  Misses        373      373              
Flag Coverage Δ
cli_func_v4 81.41% <71.42%> (-0.02%) ⬇️
py_func_v4 81.07% <71.42%> (-0.02%) ⬇️
unit 83.19% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
gitlab/mixins.py 91.54% <ø> (ø)
gitlab/__init__.py 100.00% <100.00%> (ø)
gitlab/const.py 100.00% <100.00%> (ø)

@JohnVillalovos JohnVillalovos requested a review from nejch November 17, 2021 04:06
We are planning on adding enumerated constants into gitlab/const.py,
but if we do that than they will end up being added to the top-level
gitlab namespace. We really want to get users to start using
`gitlab.const.` to access the constant values in the future.

Add the currently defined constants to a list that should not change.
Use a module level __getattr__ function so that we can deprecate
access to the top-level constants.

Add a unit test which verifies we generate a warning when accessing
the top-level constants.
Have code use constants from the gitlab.const module instead of from
the top-level gitlab module.
Update the docs to use gitlab.const to access constants.
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/const_explicit branch from ee25bcd to b3b0b5f Compare November 30, 2021 16:38
@nejch nejch merged commit e6582a3 into main Dec 1, 2021
@nejch nejch deleted the jlvillal/const_explicit branch December 1, 2021 00:13
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