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

EntityGenerator lifecycle callbacks #11621

Merged
merged 5 commits into from
Oct 5, 2024
Merged

Conversation

timum-viw
Copy link
Contributor

@timum-viw timum-viw commented Oct 2, 2024

This PR addresses a problem in the Entity Generator. when adding the same lifecycle event callback to two or more lifecycle events the generator will create a stub for each event each with the same method name. This results in fatal 'Cannot redeclare' errors. This Problem occurred only if the callback name contains uppercase letters.

this PR intends to fix this issue by updating the EntityGenerator to allow the same callback to be triggered by multiple event.

When adding the same lifecycle event callback to two or more lifecycle events, the generator will create a stub for each event resulting in fatal 'Cannot redeclare' errors. That is, only if the callback name contains uppercase letters.
this will fix the fatal errors when having the same lifecycle event callback on multiple lifecycle events. Tests will still fail, since the necessary annotations for multiple lifecycle events are not properly added.
this will fix adding annotations for all referenced lifecycle events when creating lifecycle callback stubs. It takes case insensitivity into account.
@timum-viw timum-viw marked this pull request as ready for review October 2, 2024 16:06
@greg0ire
Copy link
Member

greg0ire commented Oct 3, 2024

It seems there are CI jobs failing. Please take a look at this guide for more on how to handle those.

@timum-viw
Copy link
Contributor Author

Sorry about that. I fixed the formatting according to phpcs output and locally no errors in the changed files are reported.

fallback to parameter type detection in method body to not break extending classes
@greg0ire greg0ire added the Bug label Oct 4, 2024
@greg0ire
Copy link
Member

greg0ire commented Oct 4, 2024

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

  1. git rebase -i origin/2.20.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
  2. A window will show up with many lines, replace pick with fixup on every line but the first one
  3. Close your editor, git should do its magic, and you should end up with one commit
  4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

@greg0ire greg0ire merged commit 91709c1 into doctrine:2.20.x Oct 5, 2024
58 checks passed
@greg0ire greg0ire added this to the 2.20.0 milestone Oct 5, 2024
@greg0ire
Copy link
Member

greg0ire commented Oct 5, 2024

Argh, wrong branch… well, you will have to wait for 2.20.0 to get your bugfix. Thanks for working on that anyway, and congrats on your first contribution.

@timum-viw
Copy link
Contributor Author

Yeah, noticed that too when I read the part in the docs about picking the right branch 😆 thanks for your fast replies and your general work on doctrine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants