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.

Sign up for GitHub

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

Jump to bottom

Review the documentation regarding entity inheritance #10429

Merged
merged 8 commits into from
Jan 19, 2023
Merged

Review the documentation regarding entity inheritance #10429

merged 8 commits into from
Jan 19, 2023

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jan 19, 2023 •
edited
Loading

This PR gives the documentation chapter on inheritance a brush-up.

  • It explains concepts that apply to STI and JTI in a common section at the beginning.
  • It more precisely (IMHO) states when inheritance has to be used, where the mapping configuration has to be placed and what it needs to include.
  • Small orthographic fixups and typography fixes, no need to SHOUT AT PEOPLE.
  • Add new typos

@mpdude mpdude changed the title Review the documentation on inheritance Review the documentation regarding entity inheritance Jan 19, 2023
greg0ire
greg0ire reviewed Jan 19, 2023
View reviewed changes
docs/en/reference/inheritance-mapping.rst Outdated Show resolved Hide resolved
docs/en/reference/inheritance-mapping.rst Outdated Show resolved Hide resolved
docs/en/reference/inheritance-mapping.rst Outdated Show resolved Hide resolved
docs/en/reference/inheritance-mapping.rst Outdated Show resolved Hide resolved
docs/en/reference/inheritance-mapping.rst Outdated Show resolved Hide resolved
@mpdude mpdude mentioned this pull request Jan 19, 2023
Merged
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 19, 2023
@mpdude
Catch missing inheritance declarations early on
4c59207 
When my understanding is correct, inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

The documentation is updated accordingly at doctrine#10429.
mpdude and others added 4 commits January 19, 2023 10:04
@mpdude @greg0ire
Update docs/en/reference/inheritance-mapping.rst
074df76 
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
@mpdude @greg0ire
Update docs/en/reference/inheritance-mapping.rst
ed260d7 
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
@mpdude @greg0ire
Update docs/en/reference/inheritance-mapping.rst
25a141f 
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
@mpdude @greg0ire
Update docs/en/reference/inheritance-mapping.rst
9acc37e 
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 19, 2023
@mpdude
Catch missing inheritance declarations early on
2f476f4 
When my understanding is correct, inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

The documentation is updated accordingly at doctrine#10429.
@mpdude
Copy link
Contributor Author

mpdude commented Jan 19, 2023

All feedback addressed

@greg0ire greg0ire added this to the 2.14.2 milestone Jan 19, 2023
@greg0ire greg0ire merged commit aee1d33 into doctrine:2.14.x Jan 19, 2023
@greg0ire
Copy link
Member

Thanks @mpdude !

@mpdude mpdude deleted the review-inheritance-docs branch January 19, 2023 19:56
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 20, 2023
@mpdude
Deprecate undeclared entity inheritance
da8601b 
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

This is also pointed out in the opening comment for doctrine#8348:

> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.

Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice  and make this an exception in 3.0. The documentation is updated accordingly at doctrine#10429.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 23, 2023
@mpdude
Deprecate undeclared entity inheritance
7d7dc1b 
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

This is also pointed out in the opening comment for doctrine#8348:

> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.

Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice  and make this an exception in 3.0. The documentation is updated accordingly at doctrine#10429.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 23, 2023
@mpdude
Deprecate undeclared entity inheritance
835212f 
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

This is also pointed out in the opening comment for doctrine#8348:

> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.

Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice  and make this an exception in 3.0. The documentation is updated accordingly at doctrine#10429.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Jan 24, 2023
@mpdude
Deprecate undeclared entity inheritance
384a5b4 
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

This is also pointed out in the opening comment for doctrine#8348:

> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.

Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice  and make this an exception in 3.0. The documentation is updated accordingly at doctrine#10429.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
derrabus pushed a commit that referenced this pull request Jan 24, 2023
@mpdude
Deprecate undeclared entity inheritance (#10431)
6568782 
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.

This is also pointed out in the opening comment for #8348:

> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.

Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice  and make this an exception in 3.0. The documentation is updated accordingly at #10429.

Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.

In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
2.14.2
Development

Successfully merging this pull request may close these issues.
3 participants
@mpdude @greg0ire @derrabus