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

Conversation

@stuartmorgan-g
Copy link
Contributor

Description

Follows the structure established in #2119 to add a federated macOS implementation of the url_launcher plugin.

Related Issues

Fixes macOS portion of flutter/flutter#41721

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.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

Follows the structure established in flutter#2119 to add a federated macOS
implementation of the url_launcher plugin.

Fixes macOS portion of flutter/flutter#41721
@stuartmorgan-g
Copy link
Contributor Author

/cc @franciscojma86

@stuartmorgan-g
Copy link
Contributor Author

This doesn't include tests, since I'm not sure what the integration test strategy for federated plugins is. Should the example in the main plugin depend on the federated version and include macOS support?

@amirh
Copy link
Contributor

amirh commented Oct 14, 2019

This doesn't include tests, since I'm not sure what the integration test strategy for federated plugins is. Should the example in the main plugin depend on the federated version and include macOS support?

Interesting point, we should probably think about it some more, I think it would be nice if the platform package could provide the example's platform scaffold (e.g for Android that will be the Activity, manifest etc...) and the e2e tests that are specified in the app-facing plugin will be executed as part of the macOS CI. Though devil is in the details, and I'm not sure how we'll end up with a pubspec that has all the example app's dependencies but also a dependency on the unendorsed implementation.

cc @collinjackson

Copy link
Contributor

@amirh amirh left a comment

Choose a reason for hiding this comment

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

I haven't reviewed the swift code but the structure and docs all make sense.

We probably need some test before this can land, so either we figure out the e2e testing story, or maybe it's possible to come up with a unit test for the swift code?

@collinjackson
Copy link
Contributor

@franciscojma86 once we have driver testing of Mac apps on CI, this PR can land.

@stuartmorgan-g
Copy link
Contributor Author

Obsoleted by #2383

@stuartmorgan-g stuartmorgan-g deleted the url-launcher-macos-revisit branch April 19, 2022 17:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants