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.

Sign up for GitHub

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

Jump to bottom

Conversion to Synced Realm only supported in PBS. #3333

Merged
merged 4 commits into from
Jul 23, 2024
Merged

Conversion to Synced Realm only supported in PBS. #3333

merged 4 commits into from
Jul 23, 2024

Conversation

MongoCaleb
Copy link
Collaborator

@MongoCaleb MongoCaleb commented Jul 22, 2024 •
edited
Loading

@netlify Netlify
Copy link

netlify bot commented Jul 22, 2024 •
edited
Loading

✅ Deploy Preview for device-sdk ready!

Name Link
🔨 Latest commit 4925fac
🔍 Latest deploy log https://app.netlify.com/sites/device-sdk/deploys/669ff790922f8c00081dd95c
😎 Deploy Preview https://deploy-preview-3333--device-sdk.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 28 (🔴 down 3 from production)
Accessibility: 93 (no change from production)
Best Practices: 75 (no change from production)
SEO: 69 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 22, 2024 •
edited
Loading

Readability for Commit Hash: 4925fac

You can see any previous Readability scores (if they exist) by looking
at the comment's history.

Readability scores for changed documents:

  • source/sdk/flutter/sync: Grade Level: 10.6, Reading Ease: 47.08
  • source/sdk/swift/sync/configure-and-open-a-synced-realm: Grade Level: 7.6, Reading Ease: 65.52
  • source/sdk/dotnet/sync: Grade Level: 10.4, Reading Ease: 47.59
  • source/sdk/dotnet/sync/partition-based-sync: Grade Level: 9.7, Reading Ease: 49.41
  • source/sdk/dotnet/sync/configure-and-open-a-synced-realm: Grade Level: 8.2, Reading Ease: 64.1

For Grade Level, aim for 8 or below.

For Reading Ease scores, aim for 60 or above:

Score Difficulty
90-100 Very Easy
80-89 Easy
70-79 Fairly Easy
60-69 Medium
50-59 Fairly Hard
30-49 Hard
0-29 Very Hard

For help improving readability, try Hemingway App.

dacharyc
dacharyc requested changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@dacharyc dacharyc left a comment

Choose a reason for hiding this comment

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

Overall, LGTM - we just need to add a redirect for the page we're removing, and I have a minor suggestion re: mentioning the WriteCopy API on the .NET page.

source/sdk/dotnet/sync/configure-and-open-a-synced-realm.txt
````````````````````````````````````````````
To convert a non-synced realm to a synced realm, you can follow the process
described in :ref:`Migrate a Local-Only App to a Sync-Enabled App <realm-sync-local-to-sync>`.
The .NET SDK also provides the
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should either leave this off the page entirely, or mention here that it only works with Partition-Based Sync. Mentioning that this API exists without the warning that our "default" Sync apps can't use it feels like it will lead to user frustration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Added back in the note that was on the original page.

source/sdk/dotnet/sync/convert-realm.txt Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm down with deleting this, but we should add a redirect for it if we do.

@MongoCaleb MongoCaleb requested a review from dacharyc July 23, 2024 16:01
dacharyc
dacharyc approved these changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@dacharyc dacharyc left a comment

Choose a reason for hiding this comment

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

Looks like there is one more spot where we need to update a redirect to avoid a redirect chain, but after that change, this LGTM!

config/redirects Outdated

# DOCSP-39075
raw: ${prefix}/sdk/dotnet/sync/convert-realm -> ${deviceSDKsBase}/sdk/dotnet/sync/configure-and-open-a-synced-realm
Copy link
Collaborator

Choose a reason for hiding this comment

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

In addition to adding this new one, I see there is a prior redirect that points to the convert-realm page at line 1023 - we need to update that one, too, to avoid a redirect chain.

@dacharyc dacharyc added the Add to consolidation feature branch PR merged after cutting the consolidation feature branch, so cherry-pick to the feature branch. label Jul 23, 2024
@MongoCaleb MongoCaleb merged commit 241f2a8 into mongodb:master Jul 23, 2024
11 checks passed
@docs-builder-bot
Copy link

docs-builder-bot commented Jul 23, 2024 •
edited
Loading

✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/master/

🪵 Logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dacharyc dacharyc dacharyc approved these changes

Assignees
No one assigned
Labels
Add to consolidation feature branch PR merged after cutting the consolidation feature branch, so cherry-pick to the feature branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MongoCaleb @docs-builder-bot @dacharyc