Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Improve the type hints of synapse.api.errors.#7820

Merged
clokep merged 3 commits into
developfrom
clokep/api-error-types
Jul 14, 2020
Merged

Improve the type hints of synapse.api.errors.#7820
clokep merged 3 commits into
developfrom
clokep/api-error-types

Conversation

@clokep
Copy link
Copy Markdown
Member

@clokep clokep commented Jul 10, 2020

I started doing this in #7802 and realized it was bigger than I wanted so I split it out.

It adds more type hints to synapse.api.errors.

@clokep clokep requested a review from a team July 10, 2020 19:59
Copy link
Copy Markdown
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Does this now pass mypy synapse/api/errors.py?

Comment thread synapse/api/errors.py Outdated
Comment thread synapse/api/errors.py Outdated
@clokep
Copy link
Copy Markdown
Member Author

clokep commented Jul 13, 2020

Does this now pass mypy synapse/api/errors.py?

synapse/api is already included in mypy.

@clokep clokep requested a review from erikjohnston July 13, 2020 17:51
@erikjohnston
Copy link
Copy Markdown
Member

Does this now pass mypy synapse/api/errors.py?

synapse/api is already included in mypy.

I really expected that it would have picked up the Optional issue 😕

@clokep
Copy link
Copy Markdown
Member Author

clokep commented Jul 14, 2020

I really expected that it would have picked up the Optional issue 😕

I think mypy is "smart" in that case (when the default is None) and automatically adds Optional. Personally I find it confusing and like to be explicit.

@clokep clokep merged commit 4db1509 into develop Jul 14, 2020
@clokep clokep deleted the clokep/api-error-types branch July 14, 2020 11:04
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '4db150951':
  Improve the type hints of synapse.api.errors. (#7820)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants