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

chore: Refactor various protobuf modules #642

Closed
wants to merge 2 commits into from

Conversation

aiven-anton
Copy link
Contributor

The main goal is to increase type coverage.

The Location class is turned into a dataclass to simplify things and gain immutability. It's .get() method needs an overload to type accurately, even after simplifying its signature by raising instead of returning None when incorrect amount of arguments are given. This was addressed as a side-quest in adding types to ProtobufSchema, because it has an attribute that is assigned to from the return value of this method.

The ProtoFileElement.__eq__() is altered so that it doesn't assume the compared object is of the same type as itself, and its argument is updated to follow convention of supporting equality check with any other type of object.

ProtobufSchema.dirty is removed because it was unused.

ProtobufSchema.references along with its init argument are removed because they aren't used. This triggers a chain of removing unused arguments in a few functions.

About this change - What it does

References: #xxxxx

Why this way

First off, this fixes the issue that `Reference` objects were mutated
while acting as immutable by implementing `__hash__()`. To do that, it
breaks up the `Reference` class into two, so that instead of it having
to overloaded meanings, the unresolved state is instead represented as
its own entity, `LatestVersionReference`. This is a typical "make
illegal state unrepresentable" kind of refactoring.

Related to this, a few `list` and `dict`s are replaced with their
immutable abstract counterparts, `Sequence` and `Mapping`. This greatly
helps in reasoning about where mutation can happen, and stops me from
constantly wondering "will this list be altered when I pass this down
this chain of methods".
The main goal is to increase type coverage.

The `Location` class is turned into a dataclass to simplify things and
gain immutability. It's `.get()` method needs an overload to type
accurately, even after simplifying its signature by raising instead of
returning `None` when incorrect amount of arguments are given. This was
addressed as a side-quest in adding types to `ProtobufSchema`, because
it has an attribute that is assigned to from the return value of this
method.

The `ProtoFileElement.__eq__()` is altered so that it doesn't assume the
compared object is of the same type as itself, and its argument is
updated to follow convention of supporting equality check with any other
type of object.

`ProtobufSchema.dirty` is removed because it was unused.

`ProtobufSchema.references` along with its init argument are removed
because they aren't used. This triggers a chain of removing unused
arguments in a few functions.
@aiven-anton aiven-anton force-pushed the aiven-anton/fix/make-reference-immutable branch from 52e56b1 to 72b5b04 Compare June 1, 2023 17:19
@aiven-anton aiven-anton force-pushed the aiven-anton/fix/make-reference-immutable branch 4 times, most recently from aee3160 to ebbfba4 Compare June 13, 2023 17:57
Base automatically changed from aiven-anton/fix/make-reference-immutable to main June 16, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant