Skip to content

PHPLIB-989: Move persistable class documentation to tutorial #987

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

Merged
merged 4 commits into from
Oct 12, 2022

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Oct 4, 2022

PHPLIB-989

This moves the documentation on classes implementing MongoDB\BSON\Persistable from the BSON reference to a tutorial. It was already written as a tutorial, and linking it as a tutorial on the homepage will make it more visible to users.

I've also considered doing the same with the Type Map information, but considering that persistable classes cover typemap functionality as well, I'm not sure we'll want to. Any opinions @jmikola?

@alcaeus alcaeus self-assigned this Oct 4, 2022
@alcaeus alcaeus requested a review from jmikola October 4, 2022 08:19
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

I've also considered doing the same with the Type Map information, but considering that persistable classes cover typemap functionality as well, I'm not sure we'll want to.

The BSON reference page was always a bit of a dumping ground. I do think it'd make it makes sense to extract its sections out to other pages.

I'd suggest splitting Type Maps off to its own page or combining with the Persistable Classes tutorial page you're creating here.

Emulating the Legacy Driver can be moved to the Upgrade Guide, which I believe you already plan to rename to "Legacy Driver Upgrade Guide" (or something to that effect). For that, I'd suggest renaming BSON Type Classes to "Type Classes" and placing the "Emulating" section parallel to that under a new, top-level "BSON" heading.

That would just leave the BSON reference page with two sections: Overview and BSON Classes. From there, I'd suggest renaming "BSON Classes" to just "Classes". Those remaining two sections would be appropriate for a reference page, and then you could also add "See Also" links to the newly created tutorial page(s) and relocated "Emulating the Legacy Driver" section.

@alcaeus alcaeus force-pushed the phplib-989-serialisation-tutorial branch from 87df442 to 60f5dbb Compare October 7, 2022 06:44
@alcaeus alcaeus requested a review from jmikola October 7, 2022 09:10
@alcaeus
Copy link
Member Author

alcaeus commented Oct 7, 2022

@jmikola great suggestions - I applied them as suggested.

Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Let me know when the suggestions are applied and I'll check out the PR for a local docs build to inspect before merging. IIRC, you had some trouble with local docs builds.

@alcaeus alcaeus force-pushed the phplib-989-serialisation-tutorial branch from e83316e to 5769243 Compare October 11, 2022 08:21
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Content LGTM. I'll leave you to verify that RST build, since my Giza build environment is broken.

@jmikola
Copy link
Member

jmikola commented Oct 12, 2022

since my Giza build environment is broken.

Just fixed it and was able to preview this PR. Everything looks good.

@alcaeus alcaeus force-pushed the phplib-989-serialisation-tutorial branch from 5769243 to 5080b80 Compare October 12, 2022 07:44
@alcaeus alcaeus merged commit 1043a3a into mongodb:master Oct 12, 2022
@alcaeus alcaeus deleted the phplib-989-serialisation-tutorial branch October 12, 2022 08:46
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.

2 participants