Skip to content

PHPLIB-601: Document upcoming typing changes #985

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 8 commits into from
Oct 7, 2022

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Oct 4, 2022

PHPLIB-601

I've added an UPGRADE-1.14 document to inform users of changes they should be aware of when upgrading. Once changes for 2.0 become clearer (including a list of methods that will change their return types), we should add an UPGRADE-2.0 document to inform users of those.

@jmikola I've renamed the previous upgrade guide in the docs to "Legacy driver upgrade guide", in case we want to add a more visible documentation of method signature changes once we add BC breaks in 2.0. I've skipped a list of changes for this PR, but can list method signature changes in the upgrade document as well. An example of this is the upgrade document for doctrine/collections where interfaces were changed to add types.

We'll want to consider where to document upcoming return type changes for ext-mongodb. While users on PHP 8.1 and newer will be informed of tentative return types, users on previous version will not be aware of those changes. At the same time, as most users will consume the extension through this library (but may interact with extension classes directly), I'm not sure what the best place to document those changes are.

For now, I believe this document is sufficient to inform users of changes we've already made, as well as upcoming changes to internal classes.

@alcaeus alcaeus requested review from jmikola and levon80999 October 4, 2022 06:55
@alcaeus alcaeus self-assigned this Oct 4, 2022
@@ -281,7 +281,7 @@ inadvertent and potentially dangerous :manual:`full-document replacements
Group Command Helper
~~~~~~~~~~~~~~~~~~~~

:phpclass:`MongoDB\\Collection` does have a helper method for the
:phpclass:`MongoDB\\Collection` does not have a helper method for the
Copy link
Member

Choose a reason for hiding this comment

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

That's quite the old typo correction!

UPGRADE-1.14.md Outdated
@@ -0,0 +1,23 @@
# UPGRADE FROM 1.x to 1.14

## Method signature changes
Copy link
Member

Choose a reason for hiding this comment

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

I've skipped a list of changes for this PR, but can list method signature changes in the upgrade document as well. An example of this is the upgrade document for doctrine/collections where interfaces were changed to add types.

That format looks good to me.

We'll want to consider where to document upcoming return type changes for ext-mongodb. While users on PHP 8.1 and newer will be informed of tentative return types, users on previous version will not be aware of those changes.

Two thoughts here. Does it make sense to document all signature changes in the PHPC method changelogs? That seems like the expected place for it and would certainly be more accessible than a separate page in the PHP.net docs that might be easily overlooked.

I also wouldn't object to including PHPC API changes in this document, since those are going to arrive in the corresponding PHPC release (1.15 for PHPLIB 1.14).

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it make sense to document all signature changes in the PHPC method changelogs?

Yes. I've already added a note about tentative return types (see php/doc-en@734bafe), but calling out other changes (such as widening parameter types, etc.) should also be called out. I'll go through the changes and make sure that those are updated.

There aren't any other significant API changes for users, as we've only changed final classes in PHPC. However, it will become relevant for 2.0, at which point I'd definitely point out PHPC API changes in this upgrade document as well where it affects users.

@alcaeus alcaeus force-pushed the phplib-601-document-typing-changes branch from e981024 to 9a7ad62 Compare October 6, 2022 11:46
@alcaeus alcaeus requested a review from jmikola October 6, 2022 12:18
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.

Just a few suggestions you can apply and merge as you wish.

Take another look at the rendered Markdown file before merging, as that's how I caught some of the formatting mistakes.

@@ -19,7 +19,7 @@ Definition

.. code-block:: php

function dropDatabase($databaseName, array $options []): array|object
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

@alcaeus alcaeus merged commit ecdacff into mongodb:master Oct 7, 2022
@alcaeus alcaeus deleted the phplib-601-document-typing-changes branch October 7, 2022 06:37
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