Allow extras to be installed for typechecking #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DMRobertson
force-pushed
the
dmr/extras-for-typechecking
branch
from
b770fc0
to
72c0a02
Compare
DMRobertson
force-pushed
the
dmr/extras-for-typechecking
branch
from
72c0a02
to
412b83f
Compare
clokep
reviewed
Apr 25, 2022
| @@ -33,6 +45,9 @@ jobs: | |||
|
|
|||
| - name: Setup Poetry | |||
| uses: matrix-org/setup-python-poetry@v1 | |||
| with: | |||
| # We want to make use of type hints in optional dependencies too. | |||
| extras: "${{ inputs.typechecking-extras }}" | |||
There was a problem hiding this comment.
Seems OK -- should we use just extras for this though instead of typechecking-extras?
There was a problem hiding this comment.
I wanted to emphasise that this was only going to install extras to run mypy; flake8, isort, black all run from the "bare" set of developer dependencies that poetry defines.
Not a strongly-held opinion though; happy to change if you think extras is clearer.
There was a problem hiding this comment.
👍 Ah, I see. I think it is fine either way, just wasn't sure of the rationale!
clokep
approved these changes
Apr 27, 2022
DMRobertson
pushed a commit
that referenced
this pull request
Apr 27, 2022
DMRobertson
pushed a commit
to matrix-org/synapse
that referenced
this pull request
Apr 27, 2022
Now that matrix-org/backend-meta#6 is merged to the release/v1 branch.
DMRobertson
pushed a commit
to matrix-org/synapse
that referenced
this pull request
Apr 27, 2022
Over time we've begun to use newer versions of mypy, typeshed, stub packages---and of course we've improved our own annotations. This makes some type ignore comments no longer necessary. I have removed them. There was one exception: a module that imports `select.epoll`. The ignore is redundant on Linux, but I've kept it ignored for those of us who work on the source tree using not-Linux. (#11771) I'm more interested in the config line which enforces this. I want unused ignores to be reported, because I think it's useful feedback when annotating to know when you've fixed a problem you had to previously ignore. * Installing extras before typechecking Lacking an easy way to install all extras generically, let's bite the bullet and make install the hand-maintained `all` extra before typechecking. Now that matrix-org/backend-meta#6 is merged to the release/v1 branch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In matrix-org/synapse#12531 I was bitten by a different mypy result in CI to my local environment. CI was running
poetry install; I had runpoetry install --extras alllocally.I had intended for poetry's development environment (its list of dev-dependencies) to be sufficient for all development tasks. But that ideal doesn't work well here. On reflection, let's allow this action to install a list of extras for typechecking only.
Proof that this works: https://github.com/matrix-org/synapse/runs/6156325285?check_suite_focus=true#step:3:3 shows that we pass the extras through to
setup-python-poetry.Once merged, I'll do a minor release (1.3.0) and forward the v1 tag.