-
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-39526): Consolidate Authenticate Users page #3281
(DOCSP-39526): Consolidate Authenticate Users page #3281
Conversation
Readability for Commit Hash: 47ab62b 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.
I left a handful of relatively minor comments and suggestions. Otherwise, LGTM!
#. :ref:`Enable and Configure Authentication Providers | ||
<authentication-providers>` in the App Services documentation |
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.
The framing here makes "in the App Services documentation" read weird. It feels like we're asking readers to enable and configure providers in the documentation itself, rather than asking them to complete the procedure. We know there's a procedure in the App Services docs, but readers won't necessarily know that.
We can adjust the framing in lines 67 and 68 to make it clear we're talking about reading and completing steps in other parts of the docs, or figure out a way to make lines 71 and 72 less ambiguous. I know we generally try to warn readers when we pass them off to another docs site, but the easy answer is to remove "in the App Services documentation". 😆
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.
Ahh, gotcha. Hah, I know Cory feels pretty strongly about keeping the "in the App Services documentation" so I'll wordsmith ;)
...cludes/api-details/swift/users/authenticate-register-create-new-user-account-description.rst
Outdated
Show resolved
Hide resolved
|
||
.. include:: /includes/sdk-examples/users/authenticate-log-in-user.rst | ||
|
||
.. This tab group is only used to populate some additional details for Swift |
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.
This seems pretty awkward. I know it breaks the pattern a bit, but could we move the non-async example into the non-async description include? So, put .. include:: /includes/sdk-examples/users/authenticate-log-in-user.rst
in /includes/api-details/swift/users/authenticate-log-in-user-description.rst
.
Then we could also include the async description content in the regular description include and use the async example.
...this stuff is difficult to write about. lol. Happy to talk more if that thought isn't clear.
7cf0f51
into
mongodb:feature-consolidated-sdk-docs
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/feature-consolidated-sdk-docs/ 🪵 Logs |
Pull Request Info - SDK Docs Consolidation
Jira ticket: https://jira.mongodb.org/browse/DOCSP-39526
Staged Page
Page Source
Add links to every SDK's pages where you got the SDK-specific information:
PR Author Checklist
Before requesting a review for your PR, please check these items:
feature-consolidated-sdk-docs
branch instead ofmaster
Naming
.rst
files comply with the naming guidelinesLinks and Refs
Content
Reviewer Checklist
As a reviewer, please check these items: