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.

Sign up for GitHub

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

Jump to bottom

#11948 disallow_untyped_decorators = true #11947

Merged
merged 13 commits into from
Sep 9, 2023
Merged

#11948 disallow_untyped_decorators = true #11947

merged 13 commits into from
Sep 9, 2023

Conversation

graingert
Copy link
Member

@graingert graingert commented Sep 1, 2023 •
edited

Scope and purpose

Fixes #11948

@graingert graingert changed the title disallow_untyped_decorators = true #11948 disallow_untyped_decorators = true Sep 2, 2023
@graingert graingert marked this pull request as ready for review September 2, 2023 11:23
@chevah-robot chevah-robot requested a review from a team September 2, 2023 11:23
@graingert graingert requested a review from glyph September 3, 2023 09:46
adiroiban
adiroiban reviewed Sep 4, 2023
View reviewed changes
Copy link
Member

@adiroiban adiroiban left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for the changes.
Leaving only a comment.

I think the changes looks good but it needs a API exception

Here it is defines as "and at least three committers agree that the change is worthwhile"

I agree with such an exception

https://docs.twisted.org/en/stable/development/compatibility-policy.html#procedure-for-exceptions-to-this-policy

src/twisted/internet/base.py Show resolved Hide resolved
src/twisted/protocols/amp.py Show resolved Hide resolved
src/twisted/test/test_defer.py Show resolved Hide resolved
src/twisted/trial/_asynctest.py Outdated Show resolved Hide resolved
src/twisted/trial/_synctest.py Outdated Show resolved Hide resolved
@adiroiban adiroiban requested a review from a team September 4, 2023 12:19
@graingert
Copy link
Member Author

@glyph I'm happy to change addCleanup to # type: ignore[override] and remove the breaking change in this PR

@adiroiban adiroiban mentioned this pull request Sep 8, 2023
Open
@adiroiban
Copy link
Member

I have created this ticket #11967

I think that you can use ignore for this PR and we can look at addCleanup in a separate PR.
Please add the link to the ticket as a comment, so that we will not forget about the old ticket and avoid creating a duplicate ticket.

thanks

adiroiban
adiroiban approved these changes Sep 8, 2023
View reviewed changes
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

looks good. thanks. Only minor comments.

adiroiban
adiroiban approved these changes Sep 9, 2023
View reviewed changes
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@graingert graingert merged commit 2f5c4d8 into twisted:trunk Sep 9, 2023
22 checks passed
@graingert graingert deleted the untyped-decorator branch September 9, 2023 14:27
@clokep clokep mentioned this pull request Sep 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adiroiban adiroiban adiroiban approved these changes

@glyph glyph Awaiting requested review from glyph

Assignees
No one assigned
Labels
needs-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mypy disallow_untyped_decorators = true
4 participants
@graingert @adiroiban @glyph @chevah-robot