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.

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

Handle new package file #46

Merged
merged 3 commits into from
Apr 28, 2023
Merged

Handle new package file #46

merged 3 commits into from
Apr 28, 2023

Conversation

tothszabi
Copy link
Contributor

Checklist

  • I've read and followed the Contribution Guidelines
  • step.yml and README.md is updated with the changes (if needed)

Version

Requires a PATCH version update

Context

The e2e tests started to fail on the Linux stack. After some investigation I have found that the Dart team deprecated the way they were handling the packages and now there is a new json based variant:
dart-lang/sdk#48272

They were generously generating the old version too for backwards compatibility but after a certain version it will be completely removed.

This PR introduces handling this new json format.

Changes

The step will first try to parse the old .packages file. If it fails then it will also try to parse the new .dart_tool/package_config.json one.

Packages []struct {
Name string `json:"name"`
RootUri string `json:"rootUri"`
PackageUri string `json:"packageUri"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at https://dart.googlesource.com/sdk/+/5ae183856945440597e0674436dd89b9e08632bc/.dart_tool/package_config.json as an example, at least packageUri can be missing. Should this field rather be string? and omitempty?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

Copy link
Contributor Author

@tothszabi tothszabi Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go does not have optional fields and the way it works is that it initialises every field to the fields default value. For strings this is the "" empty string. Which is great for us and this way we do not need to care if the packageUri exists or not.

The omitempty tag is only useful for encoding. It is not used during decoding.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know what is exactly the rule here.
But in my old team we usually approved the code pro-actively allowing the author to fix comments and tests and merge without blockers.
Approved =)

AAverin
AAverin previously approved these changes Apr 27, 2023
@tothszabi tothszabi merged commit f2d76a7 into master Apr 28, 2023
@tothszabi tothszabi deleted the handle-new-package-file branch April 28, 2023 10:31
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.

3 participants