8000 [DoctrineBridge] Fixed php doc by zomberg · Pull Request #19410 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@zomberg
Copy link
Contributor
@zomberg zomberg commented Jul 23, 2016
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 ~

@Tobion
Copy link
Contributor
Tobion commented Jul 23, 2016

See fabbot CS failures

@zomberg
Copy link
Contributor Author
zomberg commented Jul 23, 2016

Done

@zomberg zomberg changed the title Fixed php doc [DoctrineBridge] Fixed php doc Jul 23, 2016
* 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
Copy link
Member

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.

Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others 8000 . Learn more.

@javiereguiluz, replaced.

Copy link
Contributor

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

Copy link
Contributor Author

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'.

Copy link
Contributor

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.

@zomberg
Copy link
Contributor Author
zomberg commented Jul 25, 2016

@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
Copy link
Member

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 ...

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@nicolas-grekas
Copy link
Member

Thank you @zomberg.

nicolas-grekas added a commit that referenced this pull request Jul 26, 2016
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
@zomberg zomberg deleted the fixed_php_doc branch July 26, 2016 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

0