RSDK-8304 - send version metadata #251
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
purplenicole730
approved these changes
Aug 1, 2024
njooma
requested changes
Aug 1, 2024
lib/src/utils.dart
Outdated
| String getVersionMetadata() { | ||
| const String sdkVersion = 'v0.0.18'; | ||
| const String apiTag = 'v0.1.328'; | ||
| return format('flutter;{};{}', sdkVersion, apiTag); |
There was a problem hiding this comment.
Don't need the format library -- you can use format strings:
return 'flutter;$sdkVersion;$apiTag'
pubspec.yaml
Outdated
| @@ -20,6 +20,7 @@ dependencies: | |||
| collection: ^1.17.1 | |||
| async: ^2.11.0 | |||
| bonsoir: ^5.1.8 | |||
| format: ^1.5.2 | |||
.github/workflows/update_protos.yml
Outdated
Comment on lines
37
to
39
| - name: Update Proto Tag | ||
| run: sed -i -e "s/apiTag = 'v[0-9]*\.[0-9]*\.[0-9]*'/api_tag = '${{ github.event.client_payload.tag }}'" > lib/src/utils.dart | ||
|
|
There was a problem hiding this comment.
Can we update the proto tag earlier, and then update the Makefile so that it reads the lib/src/utils.dart file and grabs the apiTag, and then generates that specific api tag?
njooma
approved these changes
Aug 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How it works:
update-protos.ymlupdates theapiTagvalue in thegetVersionMetadatafunctionrelease.ymlupdates thesdkVersionvalue in thegetVersionMetadatafunctionviam_clientequal to the output ofgetVersionMetadataNote that this is a bit of a bummer because we're storing the SDK version in two places (
pubspec.ymlandutils.dart). I spent a good amount of time trying to find some alternative whereby we'd get the version info out ofpubspec.ymlinstead. The potential solutions and (seemingly intractable) problems I encountered were:pubspec.ymldirectly in a synchronous way, which means that readingpubspec.ymldata would require polluting our API with async, which would be a significant breaking change and open tons of new cans of worms.pubspec.ymland convert it into a map that I can parse). This seemed promising and tests passed, but the problem I found was the absolute path topubspec.ymlcan't be reliably assumed, and setting a relative path meant the file couldn't always be found (it was found successfully for a unit test, but not by theflutter-rc-apprunning from a sibling directory). So, this approach was too fragile to work.pubspec.ymlas per the first option above. But, the function to do so is deprecated, will be removed soon, and its use is strongly discouraged. So that approach is neither future proof nor idiomatic.Given the above, I opted for the simple and consistent, but sadly duplicative, approach that we wound up with here.