-
Notifications
You must be signed in to change notification settings - Fork 88
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
(DOCSP-26313): Swift: Add a Handle Sync Errors page for SwiftUI #2524
Conversation
Readability for Commit Hash: 4663354 You can see any previous Readability scores (if they exist) by looking Readability scores for changed documents:
For Grade Level, aim for 8 or below. For Reading Ease scores, aim for 60 or above:
For help improving readability, try Hemingway App. |
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.
nice page! couple copy edits and questions on phrasing.
let dogSubscriptionExists = subs.first(named: "dogs") | ||
// Check whether the subscription already exists. Adding it more | ||
// than once causes an error. | ||
if (peopleSubscriptionExists != nil) && (dogSubscriptionExists != nil) { |
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.
It is not needed to specified != than nil, you can
if peopleSubsriptionExist && dogSubscriptionExist {
or
if let peopleSubsriptionExist = subs.first(named: "people"), let dogSubscriptionExist = subs.first(named: "dogs") {
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.
Just tried updating this syntax, but I get an error message:
Optional type 'SyncSubscription?' cannot be used as a boolean; test for '!= nil' instead
That's why I originally had the != nil in the example. This seemed like the easiest way to bypass the optional unwrapping.
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.
lgtm! nice work. a couple additional small comments.
lmk if you'd like another review after any changes from sdk team feedback.
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.
LGTM!! Just an small comment
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/master/ 🪵 Logs |
Moved the client reset handling details off the page so we can merge this PR. When PR #8109 in the realm-swift repository gets released, we can re-test and add the client reset handling details to the page. - https://jira.mongodb.org/browse/DOCSP-26313 - [Handle Sync Errors](https://preview-mongodbdacharyc.gatsbyjs.io/realm/DOCSP-26313/sdk/swift/swiftui/handle-sync-errors/) If your PR modifies the docs, you might need to also update some corresponding pages. Check if completed or N/A. - [x] Create Jira ticket for corresponding docs-app-services update(s), if any - [x] Checked/updated Admin API - [x] Checked/updated CLI reference - Swift SDK - SwiftUI/Handle Sync Errors: New page demonstrating how to set and use a sync error handler in SwiftUI. [REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md)
Pull Request Info
Moved the client reset handling details off the page so we can merge this PR. When PR #8109 in the realm-swift repository gets released, we can re-test and add the client reset handling details to the page.
Jira
Staged Changes
Reminder Checklist
If your PR modifies the docs, you might need to also update some corresponding
pages. Check if completed or N/A.
Release Notes
Review Guidelines
REVIEWING.md