Skip to content
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

Mypy #612

Merged
merged 4 commits into from
Feb 18, 2019
Merged

Mypy #612

merged 4 commits into from
Feb 18, 2019

Conversation

tpazderka
Copy link
Collaborator

  • Any changes relevant to users are recorded in the CHANGELOG.md.
  • The documentation has been updated, if necessary.

@tpazderka
Copy link
Collaborator Author

Hm, the Py34 on Windows is failing for some reason. I am considering dropping the support for Py34 as its EOL is in about a month...

@schlenk Any objections?

@schlenk
Copy link
Collaborator

schlenk commented Feb 17, 2019

@tpazderka I don't have objections about dropping Python 3.4 alongside the Python 2.7 drop, its basically the same reasoning.

@codecov-io
Copy link

codecov-io commented Feb 17, 2019

Codecov Report

Merging #612 into master will decrease coverage by 0.03%.
The diff coverage is 69.69%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #612      +/-   ##
=========================================
- Coverage   60.94%   60.9%   -0.04%     
=========================================
  Files          62      62              
  Lines       11263   11263              
  Branches     2005    2005              
=========================================
- Hits         6864    6860       -4     
- Misses       3831    3835       +4     
  Partials      568     568
Impacted Files Coverage Δ
src/oic/utils/userinfo/aa_info.py 0% <0%> (ø) ⬆️
src/oic/extension/sts.py 0% <0%> (ø) ⬆️
src/oic/utils/webfinger.py 79.38% <100%> (+0.21%) ⬆️
src/oic/utils/http_util.py 68.1% <100%> (-0.76%) ⬇️
src/oic/oic/provider.py 64.19% <100%> (+0.1%) ⬆️
src/oic/utils/client_management.py 50% <100%> (ø) ⬆️
src/oic/extension/provider.py 59.52% <100%> (ø) ⬆️
src/oic/oauth2/util.py 70.68% <100%> (ø) ⬆️
src/oic/oauth2/message.py 72% <100%> (+0.08%) ⬆️
src/oic/oauth2/provider.py 67.53% <14.28%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3752ee...ad366a6. Read the comment docs.

@CZ-NIC CZ-NIC deleted a comment Feb 17, 2019
@tpazderka
Copy link
Collaborator Author

OK, 3.4 removed as well.

for endp in self.endp:
if endp.etype == 'registration':
endpoint = urljoin(self.baseurl, endp.url)
DeprecationWarning("Using `register_endpoint` is deprecated, please use "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only create a DeprecationWarning instance? Shouldn't it be used with warning.warn(...) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, dumb mistake :)

self._srv = server

srv = property(getServer, setServer)

def __init__(self, srv, ttl=5, secure=True, httponly=True):
self.srv = None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why keep this when having the property above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

@CZ-NIC CZ-NIC deleted a comment Feb 17, 2019
Copy link
Collaborator

@schlenk schlenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tpazderka tpazderka merged commit 37a7add into master Feb 18, 2019
@tpazderka tpazderka deleted the mypy branch February 18, 2019 19:19
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