Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Add BC layer for updated constructor types#32074
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
This PR was merged into the 4.4 branch.Discussion----------Add BC layer for updated constructor types| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Reverts some CS changes done in#32066 + replaces its BC breaks by a BC layer.Our CI is too good, it bites us hard when we break our own rules :)Commits-------c34fcd9 Add BC layer for updated constructor types
| $name =''; | ||
| } | ||
| if (false !==strpos($name,$pathSeparator)) { | ||
| if (false !==strpos($name = (string)$name,$pathSeparator)) { |
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.
Changing a variable, esp it's type within a call and later rely on this is really not-obivous. It took me a while to see this.
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 understand, but this reduces merge conflicts as you know.
Tobion commentedJun 17, 2019
I don't see why those changes were necessary. If tests in other branches fail, the tests should be fixed like I did in 4.4. This is purely a test setup problem like missing mocks that don't follow the contract. |
nicolas-grekas commentedJun 17, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The CI is here to spot BC breaks, it did its job. We have bad past experiences about doing apparently minor changes but breaking real apps. Better stick to our processes. |
This PR was squashed before being merged into the 5.0-dev branch (closes #32160).Discussion----------Removed legacy code and cleanup| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Seesymfony/symfony#32074,symfony/symfony#26981labels: `HttpFoundation`, `Form`, `Security`, `SecurityBundle`, `Validator`Commits-------7b99fb45bb Removed legacy code and cleanup
This PR was squashed before being merged into the 5.0-dev branch (closes#32160).Discussion----------Removed legacy code and cleanup| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -See#32074,#26981labels: `HttpFoundation`, `Form`, `Security`, `SecurityBundle`, `Validator`Commits-------7b99fb4 Removed legacy code and cleanup
Reverts some CS changes done in#32066 + replaces its BC breaks by a BC layer.
Our CI is too good, it bites us hard when we break our own rules :)