Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
[Contributing][Code] do not distinguish regular classes and API classes#5735
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
xabbuh commentedSep 28, 2015
| Q | A |
|---|---|
| Doc fix? | no |
| New docs? | kind of (symfony/symfony#15977,symfony/symfony#15979) |
| Applies to | all |
| Fixed tickets |
stof commentedSep 29, 2015
I would just add a small note saying that these BC rules don't apply on classes/methods tagged as |
xabbuh commentedSep 29, 2015
@stof We already have this caution blocks (and a similar one for classes):
|
stof commentedSep 29, 2015
Then fine. |
wouterj commentedOct 5, 2015
👍 |
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.
We should rethink this table. In the past some columns wereYes and othersNo, so the table made sense. But now everything isYes.
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 think a table structure is very nice here. It's clear and it's consistent with the other "promises tables" below.
weaverryan commentedNov 5, 2015
Nice work Christian - thanks! |
… and API classes (xabbuh)This PR was merged into the 2.3 branch.Discussion----------[Contributing][Code] do not distinguish regular classes and API classes| Q | A| ------------- | ---| Doc fix? | no| New docs? | kind of (symfony/symfony#15977,symfony/symfony#15979)| Applies to | all| Fixed tickets |Commits-------9794d58 do not distinguish regular classes and API classes
| Reduce visibility No | ||
| Move to parent class Yes | ||
| Add argument without a default value No | ||
| Add argument with a default value No |
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.
This was a brave change. Now it's impossible to extend existing public code with new functionality. Was this intended?
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.
This was the only option we had as PHP will complain anyway (seehttps://3v4l.org/6T2DO).