-
Notifications
You must be signed in to change notification settings - Fork 10
DOCSP-51351-reorganize-toc #106
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-51351-reorganize-toc #106
Conversation
✅ Deploy Preview for docs-kotlin-sync ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔄 Deploy Preview for docs-kotlin-sync processing
|
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 with a few comments! If you have questions about my feedback lmk
:titlesonly: | ||
:maxdepth: 1 | ||
|
||
Connect with Stable API </connect/stable-api> |
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.
S: based on the TOC spreadsheet, I think the TOC label should be "Stable API" and the page title "Connect with Stable API"
Connect with Stable API </connect/stable-api> | |
Stable API </connect/stable-api> |
================================= | ||
============================== | ||
Indexes for Query Optimization | ||
============================== |
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.
S: suggestion for the Indexes section in general: there isn't much guidance about the Indexes subpages in the spreadsheet, and most drivers just have a single Indexes page without nested pages. I think it's better to avoid L3 nesting in this section, so I moved everything to L2 for PHP. example
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 have a ticket up to reorganize the indexes content: https://jira.mongodb.org/browse/DOCSP-51438 to remove L2 pages. I agree with you that we don't want L3 nesting but I think that can be done with the other ticket.
Validate Driver Signatures </validate-signatures> | ||
What's New </whats-new> | ||
Reference </reference> | ||
API Documentation </api> | ||
Issues & Help </issues-and-help> | ||
View the Source <https://github.com/mongodb/mongo-java-driver/tree/master/driver-kotlin-sync> |
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.
S: looks like this was removed from TOC
View the Source <https://github.com/mongodb/mongo-java-driver/tree/master/driver-kotlin-sync> |
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 was removed, but the node docs kept it: https://www.mongodb.com/docs/drivers/node/current/get-started/ and to my knowledge we don't want to remove content. We can discuss synchronously at our kickoff meeting!
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 with some last comments
@@ -21,9 +21,8 @@ Specialized Data Formats | |||
:titlesonly: |
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.
S: This page title can just be "Data Formats" according to the spreadsheet
Data Classes </data-formats/data-format-data-class> | ||
Kotlin Serialization </data-formats/serialization> | ||
Codecs </data-formats/codecs> | ||
BSON </data-formats/bson> |
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.
S: I think BSON is fine as the TOC label, but the actual page title might need to be longer - "Work with BSON Data"?
source/atlas-search.txt
Outdated
.. _kotlin-sync-atlas-search: | ||
|
||
================================ | ||
Run an Atlas Vector Search Query |
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:
Run an Atlas Vector Search Query | |
Run an Atlas Search Query |
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-51351
Staging Links
Self-Review Checklist