-
Notifications
You must be signed in to change notification settings - Fork 14
Rename type package to types in v1alpha8 proto files
#400
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
Rename type package to types in v1alpha8 proto files
#400
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR renames the type package to types across v1alpha8 proto definitions and updates all corresponding imports to avoid reserved keywords.
- Updated package declarations in proto files from
typetotypes. - Updated import paths and fully qualified references in proto and Python test files.
- Added a note in
RELEASE_NOTES.mddocumenting the rename.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pytests/test_common_v1alpha8.py | Updated Python imports to use types package |
| proto/frequenz/api/common/v1alpha8/types/location.proto | Changed package declaration to frequenz.api.common.v1alpha8.types |
| proto/frequenz/api/common/v1alpha8/types/interval.proto | Changed package declaration |
| proto/frequenz/api/common/v1alpha8/types/decimal.proto | Changed package declaration |
| proto/frequenz/api/common/v1alpha8/microgrid/microgrid.proto | Updated import path and type reference to types.Location |
| proto/frequenz/api/common/v1alpha8/market/price.proto | Updated import path and type reference to types.Decimal |
| proto/frequenz/api/common/v1alpha8/market/power.proto | Updated import path and type reference to types.Decimal |
| proto/frequenz/api/common/v1alpha8/market/energy.proto | Updated import path and type reference to types.Decimal |
| RELEASE_NOTES.md | Documented the renaming of the type package |
This commit renames the `type` package to `types` in the v1alpha8 proto files to avoid using reserved keywords in programming languages. Signed-off-by: Tiyash Basu <[email protected]>
d180df4 to
56f463d
Compare
llucax
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not 100% convinced about this, as it is very complicated to make sure all symbols work in all languages, or put another way, the protoc for the language should deal with these problems by renaming keywords to something that can be used in that language (or using other language hacks like these raw identifiers in rust). And if you were to use the Google APIs, you'll still have this problem.
That said, given that rust is one of the primary languages we target, I'm OK with taking the practical approach and proceed with the rename, so approving.
|
Removing from the queue in case you want to think about it 😉 |
I appreciate the goal of aligning with Google's naming conventions, but I have reservations about changing Given this, I'm inclined to stick with |
This commit renames the
typepackage totypesin the v1alpha8 proto files to avoid using reserved keywords in programming languages.