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

Add type hints #19

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

willfurnass
Copy link
Contributor

Just for fun. Won't be offended if this isn't merged.

NB type hints allowed for the identification of interesting potential issues e.g. Element.text can return None.

Copy link
Contributor

@drj11 drj11 left a comment

Choose a reason for hiding this comment

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

smdh

@@ -26,7 +26,7 @@ like Sphinx encourages.

## [Typehints](https://docs.python.org/3/library/typing.html)

* Do not use typehints.
* Please use typehints.
Copy link
Contributor

Choose a reason for hiding this comment

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

:trollface:

@drj11
Copy link
Contributor

drj11 commented Oct 21, 2020

Did you run pylint or similar as hinted in the contributing? https://github.com/RSE-Sheffield/sheffield-covid-19/blob/main/contributing.md#static-analysis

@willfurnass
Copy link
Contributor Author

Rebased on main to include commits from PR #18.

@drj11 Yes, I ran pylint: warnings from pylint are unrelated to this PR.

@drj11
Copy link
Contributor

drj11 commented Oct 26, 2020

Not compatible with the latest typelint checks (that I just merged)

@willfurnass
Copy link
Contributor Author

Rebased the PR on main so we can see the effect of the new type-hint-hating check (PR #20) when new Travis jobs run :)

@drj11
Copy link
Contributor

drj11 commented Oct 27, 2020

In a way it's quite funny:

exited with 7

@willfurnass
Copy link
Contributor Author

The use of the return code to indicate the number of type annotations is inspired!

I think this might be check mate so feel free to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants