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

Move to domainURIPrefix for FIRDynamicLinkComponents #2119

Merged
merged 11 commits into from Nov 29, 2018

Conversation

dmandar
Copy link
Contributor

@dmandar dmandar commented Nov 26, 2018

The new API is optional and current API is fully supported.

This reverts commit 46cb564.

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in a feature request so that we
    can discuss it together.

The new API is optional and current API is fully supported.

This reverts commit 46cb564.
Firebase/DynamicLinks/CHANGELOG.md Outdated Show resolved Hide resolved
Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated Show resolved Hide resolved
Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated Show resolved Hide resolved
Firebase/DynamicLinks/CHANGELOG.md Outdated Show resolved Hide resolved
Firebase/DynamicLinks/CHANGELOG.md Outdated Show resolved Hide resolved
@ryanwilson ryanwilson changed the title Revert "Revert premature api changes (#2097)" Move from domain to domainURIPrefix for FIRDynamicLinkComponents Nov 29, 2018
@ryanwilson ryanwilson changed the title Move from domain to domainURIPrefix for FIRDynamicLinkComponents Move to domainURIPrefix for FIRDynamicLinkComponents Nov 29, 2018
@@ -461,14 +462,14 @@ - (void)testLinkOptionsParamsPropertiesSetProperly {

- (void)testFDLComponentsFactoryReturnsInstanceOfCorrectClass {
NSURL *link = [NSURL URLWithString:@"https://google.com"];
id returnValue = [FIRDynamicLinkComponents componentsWithLink:link domain:kFDLURLDomain];
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, something I just realized now: it'd be good to keep a few tests around still for the old constructor to make sure we don't accidentally break it. I won't block this PR on it, but please add a few tests to make sure the old constructor isn't nil regardless of what's passed in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will add more tests for the deprecated constructor.

NSURL *link = [NSURL URLWithString:@"https://google.com/abc"];
FIRDynamicLinkComponents *components =
[FIRDynamicLinkComponents componentsWithLink:link domain:@"this is invalid domain"];
Copy link
Member

Choose a reason for hiding this comment

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

This would also be good to keep (along with the new nil test) - ensure that an invalid components with the old constructor throws an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will add more tests for the deprecated constructor.

@ryanwilson ryanwilson merged commit 36b79e6 into master Nov 29, 2018
@ryanwilson ryanwilson added this to the M39 milestone Nov 29, 2018
bstpierr added a commit that referenced this pull request Dec 6, 2018
* master: (26 commits)
  Functions Interop (#2113)
  Add a travis cron job for CocoaPod symbol collision testing (#2154)
  Save schema version on downgrade, add test to verify (#2153)
  Silence Storage Unit Test `nil` warning. (#2150)
  Update versions for Release 5.14.0 (#2145)
  gRPC: fix cases where gRPC call could be finished twice (#2146)
  Fix Swizzler test warnings (#2144)
  Update Auth CHANGELOG.md (#2128)
  Make fuzz tests optional until they pass (#2143)
  Add support of Game Center sign in (#2127)
  Add test for deprecated FDLURLComponents init API. (#2133)
  fix a typo in integration test (#2131)
  Make fuzzing less verbose to avoid exceeding Travis log limit (#2126)
  Move to `domainURIPrefix` for FIRDynamicLinkComponents (#2119)
  Carthage instructions for new gRPCCertificates.bundle location (#2132)
  Fix pod lib lint GoogleUtilities.podspec --use-libraries regression (#2130)
  Avoid using default FIROptions directly. (#2124)
  Changelog entry for LRU GC (#2122)
  Revert "Add Firebase Source to Header Search Path" (#2123)
  Custom fdl domain (#2121)
  ...
@firebase firebase locked and limited conversation to collaborators Oct 24, 2019
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.

None yet

5 participants