Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Use typed properties in tests as much as possible#51067
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
nicolas-grekas commentedJul 21, 2023
| Q | A |
|---|---|
| Branch? | 6.4 |
| Bug fix? | no |
| New feature? | no |
| Deprecations? | no |
| Tickets | - |
| License | MIT |
| Doc PR | - |
xabbuh commentedJul 21, 2023
The failures look related. |
5ccb82e to3fc02b3Comparee3ad58b to53e5180Comparenicolas-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.
PR green 🎉
Let's enforce typed properties as much as possible while reviewing PRs as of now!
| $this->dispatcher =null; | ||
| $this->factory =null; | ||
| $this->form =null; | ||
| } |
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.
let's stop adding those useless tearDown methods
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 check if the memory consumption was the same => 🎉 it's the same! 👏🏼
But there is something a bit strange. about Framework bundle (I looked at this one because it fails)
- On Github, it consumes more than 500Mb
- locally, it consumes 244.00Mb
And locally, I have less skipped tests!
For the form component, it's almost the same
53e5180 to4b35a53Comparelyrixx commentedJul 25, 2023
the following failure looks related this PR: I have the same error locally, I also tried to rebase your PR (not pushed), and got the same error On 6.4 (up to date), I don't get the error |
4b35a53 to4ad506bCompare…grekas)This PR was merged into the 7.0 branch.Discussion----------Add types to public and protected properties| Q | A| ------------- | ---| Branch? | 7.0| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -:hot_face:Allowed by#45360Follows#51068 and#51067Commits-------7ea2461 Add types to public and protected properties