Skip to content

Support create for macOS (app and plugin) #40851

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.

Sign up for GitHub

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

Jump to bottom
Merged
stuartmorgan-g merged 5 commits into from
Sep 20, 2019
Merged

Support create for macOS (app and plugin) #40851

stuartmorgan-g merged 5 commits into from
Sep 20, 2019

Conversation

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Sep 19, 2019

Description

Adds macOS support for flutter create:

  • Currently it is behind a hidden flag.
  • Adds a TargetPlatform workaround to lib/main.dart in the standard app template when enabled.
  • Supports app and plugin; module support doesn't yet exist for macOS in general.

This will eliminate the need to use FDE's examples as templates on macOS. The templates are based on the current state of FDE's examples, with templating support added (and with adoption of the new application delegate in the app, which hadn't been done yet in FDE, eliminating some boilerplate from the template).

Related Issues

Fixes #30703

Tests

I added the following tests: Tests that create with --macos creates an app or plugin with macOS support, and does not otherwise.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 19, 2019
@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Sep 19, 2019

I just realized that I didn't add macOS support to the plugin's pubspec.yaml, or the plugin example. Hold off on reviewing.

jonahwilliams
jonahwilliams approved these changes Sep 19, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Wow, we're almost there!

General approach LGTM. It's probably worth noting that 'flutter create .' won't add the override to an existing project, so I would expect more bugs to get filled here.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Sep 19, 2019

Ahh the plugins, good catch.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Sep 19, 2019

Added the new schema conditionally for now, since that's the safer approach; started a discussion in the ecosystem chat about when we want to fully flip the switch on that.

With that the plugin example just works (it just uses the app template to make the example, which is obvious in retrospect).

@codecov
Copy link

codecov bot commented Sep 19, 2019 •
edited
Loading

Codecov Report

Merging #40851 into master will decrease coverage by 2.61%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #40851      +/-   ##
==========================================
- Coverage   60.79%   58.18%   -2.62%     
==========================================
  Files         192      192              
  Lines       18401    18688     +287     
==========================================
- Hits        11187    10873     -314     
- Misses       7214     7815     +601
Flag Coverage Δ
#flutter_tool 58.18% <100%> (-2.62%) ⬇️
Impacted Files Coverage Δ
...ackages/flutter_tools/lib/src/commands/create.dart 72.96% <100%> (+0.02%) ⬆️
packages/flutter_tools/lib/src/template.dart 96.92% <100%> (-3.08%) ⬇️
packages/flutter_tools/lib/src/web/compile.dart 9.37% <0%> (-75%) ⬇️
...ages/flutter_tools/lib/src/commands/build_web.dart 33.33% <0%> (-31.38%) ⬇️
packages/flutter_tools/lib/src/run_hot.dart 39.86% <0%> (-29.21%) ⬇️
...ges/flutter_tools/lib/src/build_system/source.dart 63.51% <0%> (-22.98%) ⬇️
packages/flutter_tools/lib/src/dart/sdk.dart 80% <0%> (-20%) ⬇️
packages/flutter_tools/lib/src/linux/makefile.dart 80% <0%> (-20%) ⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart 38.28% <0%> (-18.54%) ⬇️
...ckages/flutter_tools/lib/src/reporting/events.dart 83.92% <0%> (-14.29%) ⬇️
... and 77 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 e026bd3...cb9a661. Read the comment docs.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Sep 19, 2019

Merging #40851 into master will decrease coverage by 2.61%.
The diff coverage is 100%.

codecov continues to reach... interesting conclusions about absolute coverage.

@stuartmorgan-g stuartmorgan-g merged commit 0412977 into flutter:master Sep 20, 2019
@stuartmorgan-g stuartmorgan-g deleted the macos-create-app branch September 20, 2019 00:06
@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Sep 20, 2019

I'll update flutter.dev/desktop and the FDE readmes soon to point to this so we start getting testing.

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
@stuartmorgan-g @Inconnu08
Support create for macOS (app and plugin) (flutter#40851)
04ca01a 
Adds macOS support for `flutter create`:
- Currently it is behind a hidden flag.
- Adds a TargetPlatform workaround to lib/main.dart in the standard app template when enabled.
- Supports `app` and `plugin`; `module` support doesn't yet exist for macOS in general.

This will eliminate the need to use FDE's examples as templates on macOS. The templates are based on the current state of FDE's examples, with templating support added (and with adoption of the new application delegate in the app, which hadn't been done yet in FDE, eliminating some boilerplate from the template).

Fixes flutter#30703
This was referenced Dec 5, 2020
Merged
Merged
@jmagman jmagman mentioned this pull request Dec 15, 2020
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support 'flutter create' for macOS targets

4 participants

@stuartmorgan-g @jonahwilliams @googlebot @fluttergithubbot