Skip to content

[Console] fix handling positional arguments #41952

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

Merged
merged 1 commit into from
Jul 3, 2021

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jul 2, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Follow up of #41949

@nicolas-grekas nicolas-grekas requested a review from chalasr as a code owner July 2, 2021 17:04
@carsonbot carsonbot added this to the 5.2 milestone Jul 2, 2021
@nicolas-grekas nicolas-grekas changed the base branch from 5.2 to 4.4 July 2, 2021 17:04
@nicolas-grekas nicolas-grekas modified the milestones: 5.2, 4.4 Jul 2, 2021
@carsonbot
Copy link

Hey!

I think @PatchRanger has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

I'd really appreciate a test case :)

@nicolas-grekas
Copy link
Member Author

Here you are, with a test case.

@nicolas-grekas nicolas-grekas merged commit e93f8c0 into symfony:4.4 Jul 3, 2021
chalasr added a commit to chalasr/symfony that referenced this pull request Jul 3, 2021
… (nicolas-grekas)"

This reverts commit e93f8c0, reversing
changes made to eb83be4.
nicolas-grekas added a commit that referenced this pull request Jul 4, 2021
…ents" (chalasr, nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Console] Revert "bug #41952  fix handling positional arguments"

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Reverts #41952 as it breaks tests on 5.3 due to an implicit string cast:
We added the `string` typehint to the `$name` argument in 5.x, removing it now to allow for `string|int` would be a BC break which I don't think is worth it.

Commits
-------

6ac2776 [Console] Fix type annotation on InputInterface::hasArgument()
9928be0 Revert "minor #41949 [Console] fix type annotations on InputInterface (nicolas-grekas)"
8fc58aa Revert "bug #41952 [Console] fix handling positional arguments (nicolas-grekas)"
nicolas-grekas added a commit that referenced this pull request Jul 4, 2021
* 4.4:
  Fix SkippedTestSuite
  [Console] Fix type annotation on InputInterface::hasArgument()
  Revert "minor #41949 [Console] fix type annotations on InputInterface (nicolas-grekas)"
  [EventDispatcher] Correct the called event listener method case
  Add missing translations for Japanese.
  Revert "bug #41952 [Console] fix handling positional arguments (nicolas-grekas)"
  [Security] Don't skip UserPasswordValidatorTest
  [DI] CS fix
  [Console] fix handling positional arguments
  [Validator] add translation for Vietnamese
  Add Bulgarian translation for the validator
nicolas-grekas added a commit that referenced this pull request Jul 4, 2021
* 5.2:
  CS fix
  Missing translations from traits
  Fix SkippedTestSuite
  [Console] Fix type annotation on InputInterface::hasArgument()
  Revert "minor #41949 [Console] fix type annotations on InputInterface (nicolas-grekas)"
  [EventDispatcher] Correct the called event listener method case
  Add missing translations for Japanese.
  Revert "bug #41952 [Console] fix handling positional arguments (nicolas-grekas)"
  fix backport
nicolas-grekas added a commit that referenced this pull request Jul 4, 2021
* 5.3:
  CS fix
  Missing translations from traits
  Fix SkippedTestSuite
  [Console] Fix type annotation on InputInterface::hasArgument()
  Revert "minor #41949 [Console] fix type annotations on InputInterface (nicolas-grekas)"
  [EventDispatcher] Correct the called event listener method case
  [Serializer] Need to clear cache when updating Annotation Groups on Entities
  Add missing translations for Japanese.
  Revert "bug #41952 [Console] fix handling positional arguments (nicolas-grekas)"
  Fix test
  [Security] Don't skip UserPasswordValidatorTest
  [DI] CS fix
  [Console] fix handling positional arguments
  [Validator] add translation for Vietnamese
  Add Bulgarian translation for the validator
  fix backport
  [Console] fix type annotations on InputInterface
nicolas-grekas added a commit that referenced this pull request Jul 4, 2021
* 5.4:
  [FrameworkBundle] Add missing dep
  CS fix
  Missing translations from traits
  Fix SkippedTestSuite
  [Console] Fix type annotation on InputInterface::hasArgument()
  [Notifier] add Mailjet SMS bridge
  Revert "minor #41949 [Console] fix type annotations on InputInterface (nicolas-grekas)"
  [EventDispatcher] Correct the called event listener method case
  [Serializer] Need to clear cache when updating Annotation Groups on Entities
  Add missing translations for Japanese.
  Revert "bug #41952 [Console] fix handling positional arguments (nicolas-grekas)"
  Fix test
  [Security] Don't skip UserPasswordValidatorTest
  [DI] CS fix
  [Console] fix handling positional arguments
  Add TesterTrait::assertCommandIsSuccessful() helper
  [Validator] add translation for Vietnamese
  Add Bulgarian translation for the validator
  fix backport
  [Console] fix type annotations on InputInterface
@nicolas-grekas nicolas-grekas deleted the console-int-arg branch July 6, 2021 09:05
This was referenced Jul 26, 2021
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.

3 participants