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

Set up mypy tests in tox #29

Closed
bmw opened this issue May 17, 2018 · 3 comments
Closed

Set up mypy tests in tox #29

bmw opened this issue May 17, 2018 · 3 comments
Labels
priority: unplanned Work that we believe should be done, but does not have a higher priority.

Comments

@bmw
Copy link
Member

bmw commented May 17, 2018

This should be somewhat based on Certbot's tox.ini, but will be a lot simpler because we don't have wrappers around pip and our tests here. You'll probably also have to add a mypy.ini to the root of the repo which again, should be based on the file in Certbot. You shouldn't add any check_untyped_defs for now.

You should make sure these tests pass and fix any errors that prevent them from doing so. After they're passing, .travis.yml should be configured to run them.

@bmw
Copy link
Member Author

bmw commented May 17, 2018

#28 needs to be resolved before this.

jepayne1138 added a commit to jepayne1138/josepy that referenced this issue May 17, 2018
* Implements certbot#29 but with failing mypy test
jepayne1138 added a commit to jepayne1138/josepy that referenced this issue May 17, 2018
* mypy error from previous commit was inablilty to create a consistent method resolution order for TypedJSONObjectWithFields

* determined that swapping the inheritance order on JSONObjectWithFields resolved the issue
    * my suspicion for the reason this threw an error and is resolved by this change is some conflict in MOR from abc.ABCMeta, being both inherited in JSONObjectWithFieldsMeta (metaclass for JSONObjectWithFields) and the metaclass for interfaces.JSONDeSerializable, also inherited by JSONObjectWithFields
    * I am not entirely clear as to why this solved the issue, and while I suspect it will not break anything, please review carefully

Resolves certbot#29
@bmw bmw added the priority: unplanned Work that we believe should be done, but does not have a higher priority. label Mar 25, 2020
@atombrella
Copy link
Contributor

I think this can be closed :)

@bmw
Copy link
Member Author

bmw commented Apr 23, 2021

Thanks for pointing that out!

@bmw bmw closed this as completed Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: unplanned Work that we believe should be done, but does not have a higher priority.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants