-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
RCOCOA-2305: Update base url to point to services.cloud.mongodb.com #8537
Changes from all commits
3345c95
8bba095
7c5a25d
0426ca8
ae39262
d19e8da
1b7a388
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,7 +223,7 @@ static void setOptionalString(std::optional<std::string>& dst, NSString *src) { | |
} | ||
|
||
- (NSString *)baseURL { | ||
return getOptionalString(_config.base_url); | ||
return getOptionalString(_config.base_url) ?: RLMStringViewToNSString(app::App::default_base_url()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be done by Core, and return the default url if there is no given url There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it could, but they haven't done it 😄 I think it's a fair decision on their part as they probably want to keep the struct fairly simple and have the defaults be managed elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the idea behind returning nil by default here was that most users shouldn't need to care what the base url is so we didn't even want to expose the default. The field in the core struct is optional specifically to support doing this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that most users shouldn't need to care, I am just not a huge fan of API where there's a default but it's not obvious what that is. I think returning it, even just for sanity checks is cleaner, but am open to being convinced otherwise. |
||
} | ||
|
||
- (void)setBaseURL:(nullable NSString *)baseURL { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
VERSION=10.49.1 | ||
REALM_CORE_VERSION=v14.4.1 | ||
REALM_CORE_VERSION=v14.5.0 | ||
STITCH_VERSION=8bf8ebcff6e804586c30a6ccbadb060753071a42 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the docstring should be updated to reflect that now we return the default url if there is no given base url, if we are going in that direction