Skip to content

Move to domainURIPrefix for FIRDynamicLinkComponents - #2119

Merged
ryanwilson merged 11 commits into
masterfrom
md-undocustomdomainrevert
Nov 29, 2018
Merged

Move to domainURIPrefix for FIRDynamicLinkComponents#2119
ryanwilson merged 11 commits into
masterfrom
md-undocustomdomainrevert

Conversation

@dmandar

@dmandar dmandar commented Nov 26, 2018

Copy link
Copy Markdown
Contributor

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.
Comment thread Firebase/DynamicLinks/CHANGELOG.md Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated
Comment thread Firebase/DynamicLinks/CHANGELOG.md Outdated
Comment thread Firebase/DynamicLinks/CHANGELOG.md Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/CHANGELOG.md
Comment thread Firebase/DynamicLinks/FDLURLComponents/FDLURLComponents.m Outdated
Comment thread Firebase/DynamicLinks/Utilities/FDLUtilities.m Outdated
Comment thread Firebase/DynamicLinks/FDLURLComponents/FDLURLComponents.m Outdated
Comment thread Firebase/DynamicLinks/FDLURLComponents/FDLURLComponents.m Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h Outdated
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Public/FDLURLComponents.h
Comment thread Firebase/DynamicLinks/Utilities/FDLUtilities.h Outdated
Comment thread Firebase/DynamicLinks/Utilities/FDLUtilities.m
@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

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

Copy link
Copy Markdown
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
Copy Markdown
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.

[self expectationWithDescription:@"completion called with error"];
NSURL *link = [NSURL URLWithString:@"https://google.com/abc"];
FIRDynamicLinkComponents *components =
[FIRDynamicLinkComponents componentsWithLink:link domain:@"this is invalid domain"];

Copy link
Copy Markdown
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
Copy Markdown
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.

5 participants