Skip to content

[macOS] Add missing, required dependencies to run platform_view_macos__start_up#111300

Merged
auto-submit[bot] merged 1 commit intoflutter:masterfrom
a-wallen:macos_bringup_platform_view_start_up
Sep 12, 2022
Merged

[macOS] Add missing, required dependencies to run platform_view_macos__start_up#111300
auto-submit[bot] merged 1 commit intoflutter:masterfrom
a-wallen:macos_bringup_platform_view_start_up

Conversation

@a-wallen
Copy link
Contributor

@a-wallen a-wallen commented Sep 9, 2022 •
edited
Loading

platform_view_macos__start_up was brought up in #111041, but did not have the proper dependencies to actually run the test. This PR adds the dependencies required for the platform_view example to run on macOS.

Fixes #111297 (comment)

Thank you, Jenn for catching this one.

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.

@a-wallen a-wallen force-pushed the macos_bringup_platform_view_start_up branch from d0f1176 to 5f131a0 Compare September 9, 2022 20:48
@a-wallen a-wallen force-pushed the macos_bringup_platform_view_start_up branch from 5f131a0 to d10fc33 Compare September 9, 2022 20:55
@a-wallen a-wallen requested review from cbracken and jmagman September 9, 2022 20:56
@jmagman
Copy link
Member

jmagman commented Sep 9, 2022

Can you add a link to a led run that shows it passing?

@cbracken
Copy link
Member

cbracken commented Sep 9, 2022 •
edited
Loading

(Google-internal) Details on led

@a-wallen
Copy link
Contributor Author

a-wallen commented Sep 9, 2022

Thanks @cbracken I was totally looking at the wrong page 😅

@a-wallen
Copy link
Contributor Author

a-wallen commented Sep 9, 2022 •
edited
Loading 

dependencies: >-
  [
    {"dependency": "xcode", "version": "14a5294e"}
  ]

is added, but the led run shows the failure

no developer tools were found at '/Applications/Xcode.app', requesting install. Choose an option in the dialog to download the command line developer tools.

It worked fine when I took off bringup. @CaseyHillers, do you have suggestions?

The input/output properties seem to be fine when I kick off the led run. For example,

xcode | "14a5294e"
got_revision | "d10fc3385dd8b29dddf69c7b73743f3ad91985a7"

@jmagman
Copy link
Member

jmagman commented Sep 9, 2022

The input/output properties seem to be fine when I kick off the led run. For example,

You're right, the properties look good, but it didn't actually install Xcode...

It worked fine when I took off bringup.

That would also be a fine way to test this without led

@a-wallen
Copy link
Contributor Author

a-wallen commented Sep 9, 2022 •
edited
Loading

@jmagman how would you like me to show you that this works? :)

jmagman
jmagman approved these changes Sep 10, 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.

@jmagman how would you like me to show you that this works? :)

Talked offline, removing the bringup: key in this PR and validating that it works was enough, not sure what was up with that led command not installing Xcode though.

@a-wallen a-wallen marked this pull request as ready for review September 10, 2022 03:50
@a-wallen a-wallen requested a review from keyonghan as a code owner September 10, 2022 03:50
@a-wallen a-wallen added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 12, 2022
@auto-submit auto-submit bot merged commit c6d44e2 into flutter:master Sep 12, 2022
This was referenced Sep 13, 2022
Merged
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 13, 2022
@engine-flutter-autoroll
c6d44e2 [macOS] Add missing, required dependencies to run platform_vi…
2d75654 
…ew_macos__start_up (flutter/flutter#111300)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 13, 2022
@engine-flutter-autoroll
c6d44e2 [macOS] Add missing, required dependencies to run platform_vi…
a30a5f9 
…ew_macos__start_up (flutter/flutter#111300)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 13, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 13, 2022
@engine-flutter-autoroll
c6d44e2 [macOS] Add missing, required dependencies to run platform_vi…
a8cbf22 
…ew_macos__start_up (flutter/flutter#111300)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbracken cbracken cbracken approved these changes

@jmagman jmagman jmagman approved these changes

@keyonghan keyonghan Awaiting requested review from keyonghan

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

platform_view_macos__start_up is failng

3 participants

@a-wallen @jmagman @cbracken