Skip to content

[flutter_tools] fix flutter create --offline#100941

Merged
christopherfujino merged 14 commits intoflutter:masterfrom
Jasguerrero:create_when_offline
Apr 8, 2022
Merged

[flutter_tools] fix flutter create --offline#100941
christopherfujino merged 14 commits intoflutter:masterfrom
Jasguerrero:create_when_offline

Conversation

@Jasguerrero
Copy link
Contributor

@Jasguerrero Jasguerrero commented Mar 29, 2022

Fixes: #96286

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 29, 2022
@Jasguerrero Jasguerrero changed the title add offline flag to updateAll on cache #96286 [flutter_tools] fix flutter create --offline Mar 29, 2022
@@ -171,7 +171,14 @@ class PrecacheCommand extends FlutterCommand {
}
}
if (!await _cache.isUpToDate()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think flutter precache --offline doesn't really make sense, as the point is to download pre-built binaries.

@christopherfujino
Copy link
Contributor

For testing, I would recommend adding a new test to test/commands.shard/hermetic/create_usage_test.dart and use the FakeProcessManager and FakeCommand like: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart#L122


/// Update the cache to contain all `requiredArtifacts`.
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts) async {
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, [bool offline = false]) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

for these optional parameters can you use named parameters instead?


/// Update the cache to contain all `requiredArtifacts`.
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts) async {
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, [bool offline = false]) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, [bool offline = false]) async {
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, {bool offline = false}) async {

…flutter into create_when_offline

# Conflicts:
#	packages/flutter_tools/test/commands.shard/hermetic/create_usage_test.dart
@Jasguerrero Jasguerrero marked this pull request as ready for review April 6, 2022 19:08
// ios-deploy on macOS) are required to determine `requiredArtifacts`.
await globals.cache.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal});
await globals.cache.updateAll(await requiredArtifacts);
bool offline;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool offline;
final bool offline = argParser.options.containsKey('offline') ? boolArg('offline') : false;

fs.directory(directory).childFile('.packages').createSync();
if (offline == true){
calledGetOffline += 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit. else should be on this line, after }

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot flutter create when offline

3 participants