Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] remove dependency on web_installers and transitively all code that used it #32852

Merged
auto-submit merged 1 commit into from
Aug 4, 2022
Merged

[web] remove dependency on web_installers and transitively all code that used it #32852

auto-submit merged 1 commit into from
Aug 4, 2022

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Apr 22, 2022 •
edited
Loading

Remove all dead code related to iOS Simulators and the dependency on web_installers.

@yjbanov yjbanov requested a review from jmagman April 22, 2022 00:07
@flutter-dashboard
Copy link

flutter-dashboard bot commented Apr 22, 2022

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@flutter-dashboard flutter-dashboard bot added platform-web Code specifically for the web engine needs tests labels Apr 22, 2022
jmagman
jmagman reviewed Apr 22, 2022
View reviewed changes
jmagman
jmagman approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

As a copy and removing dependency this LGTM, changing the simctl command is optional bonus (or could be done in another PR).

@yjbanov yjbanov force-pushed the remove-web-installers-dep branch from c8d1853 to 0a17597 Compare April 22, 2022 22:44
harryterkelsen
harryterkelsen approved these changes Apr 22, 2022
View reviewed changes
lib/web_ui/dev/safari_installation.dart Outdated
'available for simulators. Please use Xcode to install.');
}

// Prepate device type argument. It should look like:
Copy link
Contributor

@harryterkelsen harryterkelsen Apr 22, 2022

Choose a reason for hiding this comment

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

nit: Prepare

lib/web_ui/dev/safari_installation.dart Outdated
_booted = false;
}

/// Take the screenshot of the simulator.
Copy link
Contributor

@harryterkelsen harryterkelsen Apr 22, 2022

Choose a reason for hiding this comment

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

nit: Take a screenshot

lib/web_ui/dev/safari_installation.dart Outdated

/// Take the screenshot of the simulator.
///
/// This method takes screenshot of the entire simulator, not the
Copy link
Contributor

@harryterkelsen harryterkelsen Apr 22, 2022

Choose a reason for hiding this comment

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

nit: takes a screenshot

@Hixie
Copy link
Contributor

Hixie commented Jun 28, 2022

test-exempt: if this is just moving code between repos; if there's new code though please do test it.

@yjbanov yjbanov force-pushed the remove-web-installers-dep branch from 0a17597 to c2f998f Compare August 1, 2022 18:24
@skia-gold
Copy link

skia-gold commented Aug 1, 2022

Gold has detected about 12 new digest(s) on patchset 2.
View them at https://flutter-engine-gold.skia.org/cl/github/32852

@yjbanov yjbanov force-pushed the remove-web-installers-dep branch from c2f998f to ab609af Compare August 4, 2022 18:00
@yjbanov yjbanov changed the title [web] remove dependency on web_installers [web] remove dependency on web_installers and transitively all code that used it Aug 4, 2022
@yjbanov yjbanov force-pushed the remove-web-installers-dep branch from ab609af to 91b8aa0 Compare August 4, 2022 22:01
@yjbanov yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 4, 2022
@auto-submit auto-submit bot merged commit 7d1abfe into flutter:main Aug 4, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 5, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 5, 2022
@engine-flutter-autoroll
7d1abfe [web] remove dependency on web_installers and transitively al…
0fe96e3 
…l code that used it (flutter/engine#32852)
betrevisan pushed a commit to betrevisan/engine that referenced this pull request Aug 5, 2022
@yjbanov @betrevisan
[web] remove dependency on web_installers and transitively all code t…
c3eda39 
…hat used it (flutter#32852)
emilyabest pushed a commit to emilyabest/engine that referenced this pull request Aug 12, 2022
@yjbanov @emilyabest
[web] remove dependency on web_installers and transitively all code t…
c9d725a 
…hat used it (flutter#32852)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@jmagman jmagman jmagman approved these changes

@harryterkelsen harryterkelsen harryterkelsen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App needs tests platform-web Code specifically for the web engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yjbanov @Hixie @skia-gold @jmagman @harryterkelsen