Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DoctrineBridge] Fix theLockStoreSchemaListener#57944
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedAug 8, 2024
Hey! Thanks for your PR. You are targeting branch "7.2" but it seems your PR description refers to branch "6.4". Cheers! Carsonbot |
| $subscriber->postGenerateSchema($event); | ||
| } | ||
| publicfunctiontestPostGenerateSchemaWithInvalidLockStore() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe I'm a bit late to the party and missed an important part of the discussion of the previous PR. But… could you recap why wedelete a test for a bugfix? I would've expected anew test instead that covers the bug that we want to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@MatTheCat wrote done his reasoning here:#54407 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(I updated thetestPostGenerateSchemaLockPdo so that it would fail.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You can seetestPostGenerateSchemaWithInvalidLockStore is wrong because the function creating the generator isstatic, but its body is using$this. That means it only passes because the generator is not started and theLockStoreSchemaListener does not get any store.
f356e02 to5b05c10Compare
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
/cc@alexandre-daubois FYI
nicolas-grekas commentedAug 13, 2024
Thank you@MatTheCat. |
Uh oh!
There was an error while loading.Please reload this page.
#54407 got sidetracked
and@barton-webwings seems no longer active on GitHubso this PR takes over.