-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[pub] Log the Dart / Flutter SDK version selected #5748
Conversation
pub/lib/dependabot/pub/helpers.rb
Outdated
@@ -138,9 +144,12 @@ def ensure_right_flutter_release | |||
end | |||
|
|||
parsed = JSON.parse(stdout) | |||
flutter_version = parsed["frameworkVersion"] | |||
dart_version = parsed["dartSdkVersion"].split.first |
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.
dart_version = parsed["dartSdkVersion"].split.first | |
dart_version = parsed["dartSdkVersion"]&.split&.first |
Should we add this to safe-guard against parsed["dartSdkVersion"]
being nil? This would now raise an error if it's nil.
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.
We should always have a dart version. I made the check such that the error message will become nicer.
Thanks @sigurdm! I noticed there are a few linter violations that you'll see in the CI output, they should be easy to fix up but lmk if I can help. I also notice this breaks a (new) end-to-end test:
Which hints that this might be causing the job to fail, I left an in-line comment about what could potentially cause an issue here, but I may be off. |
Now it seems to pass checks. PTAL |
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.
This looks good to me, thanks @sigurdm!
Fix of #5396