-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Fuchsia step 1: add SDK version file and artifact download #31073
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
Fuchsia step 1: add SDK version file and artifact download #31073
Conversation
zanderso
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.
lgtm w/ possible cleanup
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.
How hard would it be to split off the web, linux, etc. artifacts into an UnstableDevelopmentArtifacts? Then you could do something like.
for (DevelopmentArtifact artifact in DevelopmentArtifacts.values) {
if (argResults[artifact.toString()]) {
requiredArtifacts.add(artifact);
}
}
if (!FlutterVersion.instance.isStable) {
for (UnstableDevelopmentArtifact artifact in UnstableDevelopmentArtifacts.values) {
if (argResults[artifact.toString()]) {
requiredArtifacts.add(artifact);
}
}
}
Or maybe put the unstable development artifacts beyond some dummy marker in DevelopmentArtifacts and then iterate over the indices of DevelopmentArtifacts.values before/after that marker.
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.
Updated DevelopmentArtifacts to be a class with a name/unstable marker, ptal?
4a2b387 to
8c1903c
Compare
zanderso
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.
still lgtm w/ a couple more nits.
| universal, | ||
| static const DevelopmentArtifact universal = DevelopmentArtifact._('universal'); | ||
|
|
||
| /// THe vaulues of DevelopmentArtifacts. |
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.
THe -> The
|
|
||
| /// A tag for a set of development artifacts that need to be cached. | ||
| enum DevelopmentArtifact { | ||
| class DevelopmentArtifact { |
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.
Maybe retain a comment here.
Description
Adds a version file to the bin/internal with the current (arbitrary) hash of the fuchsia sdk to download. Adds the ability to run
flutter precache --fuchsiaon non-stable branches.Related Issues
Fixes #31102
Tests
I added the following tests:
Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See Test Coverage.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?