feat: Move _gax functionality to _rpc#67
Conversation
Changes: - move `_gax` functionality and tests to `_rpc` - update various .sidekick.toml files - remove `_gax` tests from CI
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively moves the functionality from google_cloud_gax to google_cloud_rpc and marks the google_cloud_gax package as deprecated. The changes are comprehensive, covering code, tests, and configuration files. My review includes a few suggestions to improve code hygiene by avoiding imports of internal files from lib/src, which is a standard Dart convention. I've also noted that the README for the deprecated package could be clearer by removing the usage example. Overall, this is a solid refactoring effort.
devoncarew
left a comment
devoncarew
left a comment
There was a problem hiding this comment.
lgtm
I like the gax deprecation + re-export of rpc technique. One question in-line about the client name.
We now only have packages in generated/. Longer term, we could keep packages there, move them to packages/, or move them to something like pkgs/. The split had originally been to keep generated packages separate from hand-written ones for hygiene. Not sure if we'll end up w/ any other hand-written ones living in this repo (auth? firebase?).
| extension on JsonEncodable { | ||
| String get _asEncodedJson => jsonEncode(toJson()); | ||
| } | ||
| export 'package:google_cloud_rpc/service_client.dart'; |
There was a problem hiding this comment.
+1, exporting the symbols from here is a good idea.
| const String _clientKey = 'x-goog-api-client'; | ||
|
|
||
| // ignore: prefer_const_declarations | ||
| final String _clientName = 'gl-dart/$clientDartVersion gax/$gaxVersion'; |
There was a problem hiding this comment.
Just double checking that we still want our client name to contain 'gax' (gl-dart/0.x.0 gax/0.2.0).
There was a problem hiding this comment.
I filed an issue where I want to get this version from the sidekick configuration. It might also make sense to include the name of the actual package that we are making the call on behalf of. I need to think about that a bit.
I think that we will have hand written APIs in this repo at some point. Maybe "genai" or maybe something like pubsub. |
3 participants
Changes:
_gaxfunctionality and tests to_rpc_gax_gaxtests from CIRelies on googleapis/librarian#2713