-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[DoctrineBridge] Fixed php doc #19410
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
Conversation
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | ~ |
| License | MIT |
| Doc PR | ~ |
|
See fabbot CS failures |
|
Done |
| * hold the manager name. | ||
| * @param string $driverPattern Pattern for the metadata driver service name | ||
| * @param string $enabledParameter Service container parameter that must be | ||
| * @param bool $enabledParameter Service container parameter that must be |
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 is wrong. $enabledParameter is the parameter name (a string) or false.
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.
👍
@zomberg please replace it by string|bool
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 8000 . Learn more.
@javiereguiluz, replaced.
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.
it should be string|false
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.
@Tobion, false is not valid for this place in PHPdoc. In this place we must specify variable type but not specific value. I think correct is 'string|bool'.
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.
of course is false a valid value. See phpdoc documentation.
|
@Tobion, @nicolas-grekas, sorry, I was wrong. Replaced it by 'string|false'. |
| * @param string $cacheName The cache driver name | ||
| * @param string $objectManagerName The object manager name | ||
| * @param array $cacheDriver The cache driver mapping | ||
| * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container The ContainerBuilder instance |
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.
hum while we're at it, this could be changed to
@param ContainerBuilder ...
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.
Ok, I will fix it now.
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.
Done.
|
Thank you @zomberg. |
This PR was squashed before being merged into the 2.7 branch (closes #19410). Discussion ---------- [DoctrineBridge] Fixed php doc | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- b2d25f4 [DoctrineBridge] Fixed php doc