-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] fixed PHPDoc for setArgument/setOption in InputInterface #28374
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
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.
@liarco thanks for reporting this issue ... and for solving it yourself. Also, congrats on your first Symfony contribution!
I think I'm against being that broad. Conceptually, an argument can only be Note: it's not because something happens to work that we should make it official. That's one of these case IMHO |
@fabpot thank you for your reply, I see your point (the original proposal was exactly that "strict"). There was a reason behind the inclusion of any object implementing May I change the doc as you suggested or should I wait for further investigation? |
I think you can go ahead and make the changes. |
Thank you @liarco. |
…tInterface (liarco) This PR was squashed before being merged into the 2.8 branch (closes #28374). Discussion ---------- [Console] fixed PHPDoc for setArgument/setOption in InputInterface | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28354 | License | MIT | Doc PR | Methods now accept a value of any type except objects not implementing __toString(). **Example use case:** when using array arguments/options I can't set them programmatically without getting errors about type mismatch (from the IDE). With this patch it now works as expected. Commits ------- 61529f3 [Console] fixed PHPDoc for setArgument/setOption in InputInterface
This PR was merged into the 2.8 branch. Discussion ---------- [Console] Fix input values allowed types | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Continuation of #28374 Commits ------- 0c16cd9 [Console] Fix input values allowed types
You forgot integers, as mentioned in #28448 (comment) even the documentation examples use those. I understand the argument of type safety btw in why an int would be invalid as an option value, but in that case we do need to fix the docs, and accept that this is actually a weird B/C break in some way from the IDE or static analysis perspective (my builds started failing after upgrade from 4.1.4 to 4.1.6 because of this). |
This PR was squashed before being merged into the 2.8 branch (closes #28834). Discussion ---------- Allow integers as default console option value | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> B/C break fix for static analysis tools. Ref #28374 (comment) Commits ------- 064950a Allow integers as default console option value
Methods now accept a value of any type except objects not implementing __toString().
Example use case: when using array arguments/options I can't set them programmatically without getting errors about type mismatch (from the IDE). With this patch it now works as expected.