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

Clean up the code #11

Merged
merged 17 commits into from
Feb 29, 2024
Merged

Clean up the code #11

merged 17 commits into from
Feb 29, 2024

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Feb 28, 2024

  • Remove the MicrogridApiClient ABC
  • Rename the class to ApiClient
  • Unify the importing of protobuf generated files
  • Don't disable the check use-implicit-booleaness-not-comparison
  • Avoid disabling the unused-argument check for the whole file
  • Import grpc.aio instead of grpc
  • Improve TypeVar for ComponentData
  • Fix import formatting
  • Return Self in from_proto() constructors
  • Remove nonexistent stub from the mock
  • Add overrides to sub-classes
  • Remove unnecessary import future

llucax added 12 commits February 28, 2024 12:16
@llucax
Remove the MicrogridApiClient ABC
49d4d05 
This abstraction was never needed or used, and just adds noise.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Rename the class to ApiClient
2b92772 
Now we already have the `microgrid` namespace, so the prefix is
redundant.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Unify the importing of protobuf generated files
f944419 
We group all the necessary disabling of `pylint` checks and make the
names more clear regarding if they are from the generated protobuf
files or not.

This also reenable the checks after they are not needed anymore, so the
linter can still check the rest of the file for other (valid) errors.

We also need to use `.ValueType` instead of `.V` because `pylint` also
think that `V` doesn't exist. It seems more clear to use `.ValueType`
anyway.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Don't disable the check use-implicit-booleaness-not-comparison
d939f3f 
There is really no reason to make an exception here.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Avoid disabling the unused-argument check for the whole file
884439e 
We need to make sure to use the `disable` rules inline instead of its
own line to make sure they are not kept for the rest of the file.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Import grpc.aio instead of grpc
938c697 
Importing `grpc` doesn't always work, or confuses some linters, because
`aio` is not publicly exposed in the `grpc` package in all `grpcio`
versions.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Improve TypeVar for ComponentData
e7a1597 
It is simpler and more future proof to use a *bound* `TypeVar` instead
of specifying the `ComponentData` subclasses one by one as a
*constraint*. With this we also make sure that the resolved type is the
actual subclass of `ComponentData`.

The comment is also removed as it doesn't really provide much useful
information, all the information is either present in the `TypeVar`
declaration and documentation or is repeating the documentation for
`_component_data_task` and it is outdated because it is used in other
methods too.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Fix import formatting
bbdced7 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Return Self in from_proto() constructors
7f14064 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Remove nonexistent stub from the mock
4282adf 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Add overrides to sub-classes
1e1541f 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Remove unnecessary import future
cbfbdc9 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax llucax requested review from a team as code owners February 28, 2024 11:57
@llucax llucax requested review from thea-leake and Marenz February 28, 2024 11:57
@llucax llucax self-assigned this Feb 28, 2024
@llucax llucax added this to the v0.1.0 milestone Feb 28, 2024
@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:client Affects the client code part:docs Affects the documentation labels Feb 28, 2024
@llucax llucax added scope:breaking-change Breaking change, users will need to update their code type:enhancement New feature or enhancement visitble to users type:tech-debt Improves the project without visible changes for users and removed part:docs Affects the documentation labels Feb 28, 2024
@llucax
Rename module metadata to _metadata
b852302 
All the other modules are private, so there is no reason why this
shouldn't be either.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Convert Connection to a dataclass
43d54e6 
All other classes are dataclasses, so it is better for consistency.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Export public symbols via frequenz.client.microgrid
5694504 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
Make functions in private module public
8fa9081 
The module itself is private, but the functions are used outside the
module, so they should be public.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@github-actions github-actions bot added the part:docs Affects the documentation label Feb 28, 2024
@llucax
Prepare release notes for the v0.1.0 release
15ace20 
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax llucax mentioned this pull request Feb 28, 2024
Merged
@llucax llucax added this pull request to the merge queue Feb 29, 2024
Merged via the queue into frequenz-floss:v0.x.x with commit 4f9a5a1 Feb 29, 2024
14 checks passed
@llucax llucax deleted the cleanups branch February 29, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Marenz Marenz Marenz approved these changes

@thea-leake thea-leake Awaiting requested review from thea-leake thea-leake is a code owner automatically assigned from frequenz-floss/api-microgrid-team

Assignees

@llucax llucax

Labels
part:client Affects the client code part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests scope:breaking-change Breaking change, users will need to update their code type:enhancement New feature or enhancement visitble to users type:tech-debt Improves the project without visible changes for users
Projects
None yet
Milestone
v0.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@llucax @Marenz