[flutter_tools] remove pub dependencies from universal#97722
Conversation
flutter-dashboard
bot
added
the
tool
christopherfujino
changed the title
|
FYI I removed the issue number from the PR description, as GitHub already adds the PR number to the description, and having both was confusing. When GitHub merges PRs it uses the description as the commit message. |
…ub.com/Jasguerrero/flutter into remove_pub_dependencies_from_universal
|
@Jasguerrero looks like you made a bad git merge, due to the size of the diff. |
Jasguerrero
force-pushed
the
remove_pub_dependencies_from_universal
branch
from
95ac76f to
042e52c
Compare
| _artifacts.add(artifactSet); | ||
| } | ||
|
|
||
| List<ArtifactSet> getArtifactSet() => _artifacts; |
There was a problem hiding this comment.
By making a public getter for this private field, you've essentially made it public. I don't think you actually need to add this method, see my comments below on the cache_test
| expect(() => cache.storageBaseUrl, throwsToolExit()); | ||
| }); | ||
|
|
||
| testWithoutContext('PubDependencies should be registered as web based', () async { |
There was a problem hiding this comment.
A simpler test would be:
final PubDependencies pubDependencies = PubDependencies(
flutterRoot: () => '',
logger: logger,
pub: () => FakePub(),
);
expect(pubDependencies.developmentArtifact, DevelopmentArtifact.web);Then you wouldn't need to add the public method to get a private field from the Cache class.
| ...noColorTerminalOverride, | ||
| }); | ||
|
|
||
| testUsingContext('create --offline success', () async { |
There was a problem hiding this comment.
Sorry, I should have caught it before, but I believe that this integration test is not needed because we already have: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/commands.shard/permeable/create_test.dart#L1871
3 participants
#96286
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.