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.

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-29036: Generate staging link #7

Merged
merged 42 commits into from
Apr 10, 2024

Conversation

norareidy
Copy link
Collaborator

@norareidy norareidy commented Apr 1, 2024

Pull Request Info

Note to reviewer: the individual files have already been reviewed, but this PR is the first to include a fully staged site. As a result, it addresses issues that were missed in previous PRs and surfaced during staging. I'm mainly looking for a review of the staging link to make sure everything looks okay.

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-29036
Staging - https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link
Collaborator

@caitlindavey caitlindavey left a comment

Choose a reason for hiding this comment

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

LGTM - Few tweaks here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This sentence looks like it has formatting errors:
A :refson_oid_t` that should not be modified or freed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The images appear to be broken when viewing this page in staging.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The toggles in the bottom feel like they are broken and keep kicking back to the Visual Studio page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm they seem to be working for me. So both the left and right toggles bring you to the visual studio guide?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The toggle at the bottle had an incorrect link.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Which one? I'm seeing "Bulk Write Operations" (left) and "Aggregation Framework examples" (right), with both linking to the correct page

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

I grouped feedback in one comment since adding comments to files was slow. Here is feedback after skimming the stage links:

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libbson/
Replace link to www.mongoc.org? I expect the new reference docs landing page will be at a mongodb.com URL similar.

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libbson/api/bson_t/bson_append_oid/
Fix format of :refson_t

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libbson/tutorials/include-and-link/
Add :language: c to code sample. Sample code for hello_bson.c does not appear to show syntax highlighting. Compare with https://mongoc.org/libbson/current/include-and-link.html. Same suggestion applies to other ..literal-include code samples.

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libmongoc/tutorials/obtaining-libraries/from-source/

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libmongoc/tutorials/obtaining-libraries/installing/
Remove extra backticks around CMakeLists.txt in code snippet caption.

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libmongoc/howto/source-install/

  • Remove extra backticks around pkg-config in code snippet caption.
  • Remove <cmake--install.--config> and <cmake.--build>

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libmongoc/guides/
Remove Client-Side Field Level Encryption. Client-Side Field Level Encryption is already nested under In-Use Encryption. This results in the page showing twice in the left-hand navigation.

snooty.toml Outdated Show resolved Hide resolved
generated ``IMPORTED`` targets:

.. code-block:: cmake
:caption: `CMakeLists.txt`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
:caption: `CMakeLists.txt`
:caption: CMakeLists.txt

@@ -78,7 +78,7 @@ metadata.
default, use the following command:

.. code-block::
:caption: Ask ``pkg-config`` what directories it will search by default
:caption: Ask `pkg-config` what directories it will search by default
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
:caption: Ask `pkg-config` what directories it will search by default
:caption: Ask pkg-config what directories it will search by default

snooty.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM with backtick removals (commented in files) and this change:

https://preview-mongodbnorareidy.gatsbyjs.io/c/DOCSP-29036-staging/libmongoc/tutorials/obtaining-libraries/from-source/
Remove the <cmake.-G> <cmake.-T> and <cmake.-A> in the footnotes. Those were previously used by Sphinx to link to CMake documentation.

@norareidy norareidy merged commit 080884e into mongodb:master Apr 10, 2024
0 of 2 checks passed
@norareidy norareidy deleted the DOCSP-29036-staging branch April 10, 2024 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants