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

Mark all packets as final #53

Merged
merged 1 commit into from
Mar 1, 2023
Merged

Mark all packets as final #53

merged 1 commit into from
Mar 1, 2023

Conversation

ItsDrike
Copy link
Member

@ItsDrike ItsDrike commented Mar 1, 2023

The packet classes weren't marked with typing.final, which meant the existence of abstract methods wasn't enforced, as a type checker couldn't know whether this wasn't just an extension for the ABC, that's still abstract.

By explicitly marking these with the @final decorator, these packets will now be treated as the final (concrete) classes, and implementation of all abstract methods will be enforced.

Marking these packets as final will also prevent subclassing them (type-wise), meaning any subclassing will need # type: ignore. This does make sense, no matter what decision will be made in #45, as subclassing older packet classes from different versions, to avoid code repetition is LSP breaking, and so a need for a type ignore would be justified there anyway.

@ItsDrike ItsDrike added t: bug Something isn't working p: 2 - normal Normal priority a: packets Related to packets (packet classes, or their reading/writing) labels Mar 1, 2023
@ItsDrike ItsDrike merged commit d7d8277 into main Mar 1, 2023
@ItsDrike ItsDrike deleted the make-packets-final branch March 1, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: packets Related to packets (packet classes, or their reading/writing) p: 2 - normal Normal priority t: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant