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

Restructure resident web runner usage to avoid SDK users that don't support dwds #37815

Merged
merged 5 commits into from
Aug 8, 2019

Conversation

jonahwilliams
Copy link
Member

Description

dwds doesn't exist in google3, which means I'll need to restructure the code so we can exclude it from the internal tool. To avoid a revert/reland/revert cycle I can remove the imports for now

@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 7, 2019
@codecov
Copy link

codecov bot commented Aug 7, 2019

Codecov Report

Merging #37815 into master will increase coverage by 0.2%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #37815     +/-   ##
=========================================
+ Coverage   55.52%   55.72%   +0.2%     
=========================================
  Files         193      194      +1     
  Lines       18168    18172      +4     
=========================================
+ Hits        10087    10127     +40     
+ Misses       8081     8045     -36
Flag Coverage Δ
#flutter_tool 55.72% <0%> (+0.2%) ⬆️
Impacted Files Coverage Δ
...ges/flutter_tools/lib/src/build_runner/web_fs.dart 24.24% <ø> (ø)
packages/flutter_tools/lib/executable.dart 92.85% <0%> (-2.27%) ⬇️
packages/flutter_tools/lib/src/commands/run.dart 60.09% <0%> (ø) ⬆️
...ackages/flutter_tools/lib/src/commands/daemon.dart 34.18% <0%> (+12.37%) ⬆️
...ools/lib/src/build_runner/resident_web_runner.dart 68.5% <0%> (ø)
packages/flutter_tools/lib/src/web/web_runner.dart 0% <0%> (ø)
packages/flutter_tools/lib/src/base/logger.dart 57.63% <0%> (-24.81%) ⬇️
packages/flutter_tools/lib/src/version.dart 90.73% <0%> (-1.96%) ⬇️
packages/flutter_tools/lib/src/cache.dart 41.99% <0%> (-0.73%) ⬇️
packages/flutter_tools/lib/src/vmservice.dart 37.15% <0%> (-0.16%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b39e315...daf5536. Read the comment docs.

@jonahwilliams jonahwilliams changed the title Disable resident web runner usage Restructure resident web runner usage to avoid google3 dependency issue Aug 8, 2019
@jonahwilliams
Copy link
Member Author

cc @yjbanov PTAL

@jonahwilliams
Copy link
Member Author

cc @darrenaustin

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

So the messages are not confusing to non-google3 users, let's refer to it as "SDK users that don't support dwds" in the PR description and comments.

@jonahwilliams jonahwilliams changed the title Restructure resident web runner usage to avoid google3 dependency issue Restructure resident web runner usage to avoid SDK users that don't support dwds Aug 8, 2019
@jonahwilliams jonahwilliams merged commit f98df59 into flutter:master Aug 8, 2019
@jonahwilliams jonahwilliams deleted the restructure_of_imports branch August 8, 2019 23:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

4 participants