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

Address split of doctrine/common #11007

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

greg0ire
Copy link
Member

doctrine/common has been split in several packages. A lot of what was true about doctrine/common is true about doctrine/persistence today, so let us simply reuse the existing paragraphs and mention persistence instead of common.

- ORM (includes DBAL+Common)
- Persistence
- DBAL
- ORM (includes DBAL+Persistence)
Copy link
Member

Choose a reason for hiding this comment

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

I would say "depends on" rather than "includes"

~~~~~~~~~~~~~~~~~~

The Common package contains highly reusable components that have no
The Persistence package contains highly reusable components that have no
Copy link
Member

Choose a reason for hiding this comment

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

This sentence probably refers more to doctrine/collections than doctrine/persistence. Maybe it should be mentioned as well.

- DBAL (includes Common)
- ORM (includes DBAL+Common)
- Persistence
- DBAL
Copy link
Member

Choose a reason for hiding this comment

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

As those are about separate packages that have their own docs, should we have links to their docs ?

- `Collections <https://www.doctrine-project.org/projects/doctrine-collections/en/stable/index.html>`_
- `Persistence <https://www.doctrine-project.org/projects/doctrine-persistence/en/stable/index.html>`_
- `DBAL <https://www.doctrine-project.org/projects/doctrine-dbal/en/stable/index.html>`_
- ORM (depends on DBAL+Persistence)
Copy link
Member

Choose a reason for hiding this comment

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

and also on collections

@greg0ire greg0ire requested a review from stof October 14, 2023 19:09
is split in to these packages for a few reasons and they are to...


- ...make things more maintainable and decoupled
- ...allow you to use the code in Doctrine Common without the ORM
- ...allow you to use the code in Doctrine Persistence without the ORM
Copy link
Member

Choose a reason for hiding this comment

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

… and Collections?

Comment on lines 47 to 49
The Collection and Persistence package contains highly reusable
components that have no dependencies beyond the package itself (and PHP,
of course). The root namespace of the Persistence package is
Copy link
Member

Choose a reason for hiding this comment

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

Persistence has a dependency to the Event-Manager. I'm not sure what this paragraph was supposed to mean back then and now. That these packages are lightweight without a dependency chain?

Copy link
Member

Choose a reason for hiding this comment

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

I guess this paragraph was true for common: at that time, event-manager was part of common as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I should use the plural, which will make the sentence "beyond the package themselves". If I add Persistence to the paragraph, this "themselves" will be vague enough that it can mean "each other" I think.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me 😄

doctrine/common has been split in several packages. A lot of what was
true about doctrine/common is true about doctrine/persistence today, so
let us simply reuse the existing paragraphs and mention persistence
instead of common.
@greg0ire greg0ire added this to the 2.16.3 milestone Oct 17, 2023
@greg0ire greg0ire merged commit 3676e3c into doctrine:2.16.x Oct 17, 2023
@greg0ire greg0ire deleted the refresh-archi-docs branch October 17, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants