Skip to content

DOP-3559#786

Merged
crisatmongo merged 4 commits into
masterfrom
DOP-3559
May 2, 2023
Merged

DOP-3559#786
crisatmongo merged 4 commits into
masterfrom
DOP-3559

Conversation

@crisatmongo
Copy link
Copy Markdown
Contributor

Deprecated CSSClass component removed from Component Factory, Document Body, and Document Body Preview. Component directory and subdirectories also removed.

Stories/Links:

DOP-3559

Copy link
Copy Markdown
Collaborator

@mmeigs mmeigs left a comment

Choose a reason for hiding this comment

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

Looks good! Just going to defer to @rayangler on the A/C in the ticket stating:
Usage of cssclass directive will yield a "not yet implemented" warning
This warning might be more useful within snooty-parser, but either way it's something to be checked before approval.

@rayangler
Copy link
Copy Markdown
Contributor

This warning might be more useful within snooty-parser, but either way it's something to be checked before approval.

@mmeigs That A/C was in reference to the Component Factory's existing warning. I believe since the directive is only marked as "deprecated" by the parser, there's a chance a node can make it into the AST. In this case, we want to warn to the user that we're intentionally not rendering anything in the frontend.

If a directive does not exist in the parser, then it will print out an appropriate error. Will double-check this behavior later when I get the chance!

@mmeigs mmeigs self-requested a review March 22, 2023 16:03
mmeigs
mmeigs previously approved these changes Mar 22, 2023
Copy link
Copy Markdown
Collaborator

@mmeigs mmeigs left a comment

Choose a reason for hiding this comment

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

@rayangler was completely right. The parser emits a warning, and now snooty also emits a warning. Nice!

'card-group': CardGroup,
chapter: Chapter,
chapters: Chapters,
class: CSSClass,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't even know that the class directive was mapped to the CSSClass component.

@schmalliso should we actually keep the CSSClass component alive for the class directive until DOP-2317 is resolved to avoid potential conflicts with docs that still use .. class:: hidden?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UUUGH. Let me figure out how many instances of class are actually "live" in the content and see if we can get rid of that while we're at it. The writer who was passionate about the hiding is no longer at MongoDB so we may be able to move past it. I'll try to get an answer by tomorrow.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So I think this is all of the .. class:: hidden instances in the main branches of the docs repos

.//docs-mongodb-shell/source/reference.txt:.. class:: hidden
.//docs-mongodb-shell/source/snippets.txt:.. class:: hidden
.//docs-mongodb-shell/source/crud.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs-app-services/source/functions/mongodb/api.txt:.. class:: hidden
.//docs/source/reference/read-concern-snapshot.txt:.. class:: hidden
.//docs/source/reference/read-concern-available.txt:.. class:: hidden
.//docs/source/reference/read-concern-local.txt:.. class:: hidden
.//docs/source/reference/limits.txt:.. limit:: Hidden Indexes

Which is really not that many - a lot are on the app services functions/mongodb/api page handling methods. It seems plausible that we can work with content to find a better solution here, but lets not merge until we reach consensus.

@mmeigs mmeigs self-requested a review March 22, 2023 18:39
@mmeigs mmeigs dismissed their stale review March 22, 2023 18:40

more details needed

@schmalliso
Copy link
Copy Markdown
Contributor

@crisatmongo Can you rebase this so we can merge it in, please?

…y, Document Body, and Document Body Preview. Component directory and subdirectories also removed.
@crisatmongo crisatmongo merged commit ba5cf24 into master May 2, 2023
@crisatmongo crisatmongo deleted the DOP-3559 branch May 2, 2023 17:00
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.

4 participants