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

Disallow untyped defs by default #11302

Closed
wants to merge 3 commits into from

Conversation

callahad
Copy link
Contributor

@callahad callahad commented Nov 10, 2021

Not the prettiest mypy configuration, but it's something.

The first commit message in this PR may be of interest.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

This is mainly to take advantage of the significantly more convenient
syntax for module overrides. For example, instead of:

    [mypy-synapse.api.*]
    disallow_untyped_defs = True

    [mypy-synapse.app.*]
    disallow_untyped_defs = True

    [mypy-synapse.crypto.*]
    disallow_untyped_defs = True

The pyproject.toml file allows:

    [[tool.mypy.overrides]]
    module = [
        "synapse.api.*",
        "synapse.app.*",
        "synapse.crypto.*",
    ]
    disallow_untyped_defs = true

Not a huge difference with two or three modules, but we list many more.

- For disallow_untyped_defs, this uses 47 lines instead of 129.
- For ignore_missing_imports, this uses 34 lines instead of 90.

This is especially important as I'm about to set disallow_untyped_defs
at the global level and provide module-level opt-outs so that we require
typed definitions in all newly added code.

Signed-off-by: Dan Callahan <[email protected]>
Signed-off-by: Dan Callahan <[email protected]>
@callahad callahad requested a review from a team as a code owner November 10, 2021 23:52
@clokep
Copy link
Member

clokep commented Nov 11, 2021

I find it very confusing when there's major refactorings done at the same time as changes. Can we split this into two PRs so we can discuss the changes separately:

  1. Refactoring mypy.ini -> pyproject.toml (personally I'm not a fan of this, I like each tool to have a separate config file...)
  2. Enabling untyped defs everywhere. (I'm surprised there's so few! 🎉 )

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Per my comment.

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