Skip to content

[HttpFoundation] Add method getString to ParameterBag #46245

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

Closed
wants to merge 13 commits into from

Conversation

SHTIKOV
Copy link

@SHTIKOV SHTIKOV commented May 4, 2022

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

Added method getString to ParameterBag for use strict type. We really need this method to make it easier to work with strong typing, because the get method returns several data types, and we need a specific one.

Reopen old PR #44787, I closed by my mistake...

@carsonbot carsonbot added this to the 6.1 milestone May 4, 2022
@SHTIKOV SHTIKOV changed the title Extend parameter bag [HttpFoundation] Add method getString to ParameterBag May 4, 2022
@carsonbot
Copy link

Hey!

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

Cheers!

Carsonbot

@fabpot fabpot modified the milestones: 6.1, 6.2 May 8, 2022

* Add method `convertString` to `ParameterBag`
* Rename method `getInt` to `convertInt` in `ParameterBag`

Choose a reason for hiding this comment

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

maybe add deprecate convertInt ?

Copy link
Author

Choose a reason for hiding this comment

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

but convertInt is not deprecated...?

Choose a reason for hiding this comment

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

I meant getInt()

@SHTIKOV
Copy link
Author

SHTIKOV commented Jun 8, 2022

@fabpot , WDT?)

@nicolas-grekas
Copy link
Member

Closing in favor of #44787, which I'm reopening because there are useful comments there that shouldn't be lost.

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.

5 participants