Skip to content

Conversation

@ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Oct 19, 2017

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #23888 (comment)
License MIT
Doc PR symfony/symfony-docs#...

cc @nicolas-grekas

@fabpot
Copy link
Member

fabpot commented Oct 19, 2017

That's a BC break.

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 19, 2017

Against a BETA release? Affected code is a new 3.4 feat 🤔

@nicolas-grekas
Copy link
Member

I would consider it as a bug fix also.

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 19, 2017

Right, the bugfix in disguise is calling setProvidedTypes(). Following gettype() names is just something i think we need to choose now and be future proof (or at least is the desired behavior we want).

@jvasseur
Copy link
Contributor

jvasseur commented Oct 19, 2017

I think we should keep the current types,int and bool were chosen as the "correct" way of representing those types when scalar type hints were added to PHP. They are also the the standard way of using is_* function (even if is_integer exists as an alias).

The problem is typeof returning non-standard type names. But that's not something that should imply changes in Symfony, it's more something that should be fixed in PHP (by deprecating gettype and introduction a replacement that return the correct strings).

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 19, 2017

Hm good point. We dont respect gettype() returning "double" either.

👍 for shortnames from me.

@nicolas-grekas
Copy link
Member

Status: needs work

@ro0NL ro0NL changed the title [DI] Follow type names from gettype() in env var provided types [DI] Register default env var provided types Oct 21, 2017
@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 21, 2017

Status: needs review

cc @jvasseur

@fabpot
Copy link
Member

fabpot commented Oct 23, 2017

Thank you @ro0NL.

@fabpot fabpot merged commit 3cee7a6 into symfony:3.4 Oct 23, 2017
fabpot added a commit that referenced this pull request Oct 23, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Register default env var provided types

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23888 (comment)
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->

cc @nicolas-grekas

Commits
-------

3cee7a6 [DI] Register default env var provided types
@ro0NL ro0NL deleted the di/env-types branch October 23, 2017 15:42
This was referenced Oct 30, 2017
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