Skip to content

Use enumeration for LibraryLocation and LibraryLayout fields in grpc API #585

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

Closed
wants to merge 2 commits into from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Feb 18, 2020

The enumeration is also reused inside the arduino package to reduce code duplication.

This kind of change IMHO improves:

  • code duplication, because the enumeration is defined in a single place
  • the old string field in grpc definition, because now is an enumeraion and not a free string: t's more clear which value it can assume
  • language bindings. In particular in the Java bindings the generated API is better, for example see arduino/Arduino@9f58bae

As a side effect, after this change, the rpc definitions starts to spill inside the arduino package.
This means that the rpc definitions inside the .proto files became a sort of tables-of-truth, at least for this part of the API.

cmaglie added 2 commits March 2, 2020 17:52
The enumeration is also reused inside the arduino package to reduce
duplication.
The enumeration is also reused inside the arduino package to reduce
code duplication.
@cmaglie cmaglie changed the title Use enumeration for LibraryLocation field in grpc API Use enumeration for LibraryLocation and LibraryLayout fields in grpc API Mar 3, 2020
@cmaglie cmaglie self-assigned this Mar 3, 2020
@cmaglie
Copy link
Member Author

cmaglie commented Mar 11, 2020

Superseded by #613

@cmaglie cmaglie closed this Mar 11, 2020
@cmaglie cmaglie deleted the grpc-lib-enums branch March 11, 2020 14:52
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.

1 participant