Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Config] Allow scalar configuration in PHP Configuration#46328

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 2 commits intosymfony:5.4fromHypeMC:scalars-in-config-builder
May 17, 2022

Conversation

HypeMC
Copy link
Member

QA
Branch?5.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFixsymfony/monolog-bundle#417 (comment)
LicenseMIT
Doc PR-

Fixes passing scalar values to array nodes that have abeforeNormalization hook, eg:

returnstaticfunction (MonologConfig$config):void {$config->handler('console')// ...        ->processPsr3Messages()            ->enabled(true)    ;};

can be shortened thanks to abeforeNormalization hook:

returnstaticfunction (MonologConfig$config):void {$config->handler('console')// ...        ->processPsr3Messages(true)    ;};

I've used some of the code from#44166 by@jderusse. Since his PR is a feature it can't go on 5.4, but it still helped a lot.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Is there a way to do this only when the node actually has a beforeNormalization? Unless I missed something, right now this will always generate the adaptative code while it might not be needed when beforeNormalization isn't defined.

@HypeMC
Copy link
MemberAuthor

Is there a way to do this only when the node actually has a beforeNormalization? Unless I missed something, right now this will always generate the adaptative code while it might not be needed when beforeNormalization isn't defined.

@nicolas-grekas As far as I can tell there's no way to currently know if there are any$normalizationClosures set or not, ahasNormalizationClosures method could be added to theBaseNode, but that would be a new feature if I'm not mistaking.
Another way would be by using the reflection api.

@nicolas-grekas
Copy link
Member

Let's do it by reflection then: that's within the same package, that's fine.

HypeMC and ging-dev reacted with thumbs up emoji

@HypeMCHypeMCforce-pushed thescalars-in-config-builder branch from66a8969 tobe1071bCompareMay 15, 2022 17:16
@HypeMC
Copy link
MemberAuthor

Let's do it by reflection then: that's within the same package, that's fine.

@nicolas-grekas The adaptive code is now conditionally generated depending on whether there are normalization closures set or not. I added a new method to theBuilder/Property class, I think this shouldn't be a problem since it's an internal class, but even if it is a problem, the code can easily be moved to a private method inConfigBuilderGenerator, it's just a little cleaner this way.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Good one, thank you!

@nicolas-grekasnicolas-grekasforce-pushed thescalars-in-config-builder branch from9d16892 to2d81a3aCompareMay 17, 2022 10:40
@nicolas-grekas
Copy link
Member

Thank you@HypeMC.

@nicolas-grekasnicolas-grekas merged commit56c6dc1 intosymfony:5.4May 17, 2022
@nicolas-grekas
Copy link
Member

And thank you@jderusse also!

@HypeMCHypeMC deleted the scalars-in-config-builder branchMay 17, 2022 10:41
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedMay 17, 2022•
edited
Loading

I've merged this up into 6.0 but there is a failing test:

  1. Symfony\Component\Config\Tests\Builder\GeneratedConfigTest::testConfig with data set "ScalarNormalizedTypes" ('ScalarNormalizedTypes', 'scalar_normalized_types')
    TypeError: Symfony\Config\ScalarNormalizedTypesConfig::simpleArray(): Argument#1 ($value) must be of type Symfony\Component\Config\Loader\ParamConfigurator|array, string given, called in src/Symfony/Component/Config/Tests/Builder/Fixtures/ScalarNormalizedTypes.config.php on line 16

Could you please have a look and send a fix? 🙏

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedMay 17, 2022•
edited
Loading

I'd be also happy to let you merge 6.0 into 6.1 if you're up to 😬 (you can do it on your fork and let me know the commit hash here.)

@HypeMC
Copy link
MemberAuthor

@nicolas-grekas Sure, I'll take a look.

nicolas-grekas added a commit that referenced this pull requestMay 17, 2022
This PR was merged into the 6.0 branch.Discussion----------[Config] Fix PHP Configuration type hints & tests| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#46328 (comment)| License       | MIT| Doc PR        | -Followup to#46328Commits-------9d38089 [Config] Fix PHP Configuration type hints & tests
@HypeMC
Copy link
MemberAuthor

I'd be also happy to let you merge 6.0 into 6.1 if you're up to grimacing (you can do it on your fork and let me know the commit hash here.)

@nicolas-grekas DoneHypeMC@123b165

@nicolas-grekas
Copy link
Member

Great thank you so much! Now into 6.1.

This was referencedMay 27, 2022
nicolas-grekas added a commit that referenced this pull requestOct 24, 2022
…russe)This PR was merged into the 6.2 branch.Discussion----------[Config] Use better typehint in PHP Configuration| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This PR now followup#46328 to replace the generic `mixed` type-hint by type inferred from normalizations (ie, if a config use `ifString`, when now that the normalization works only for string, these `mixed` can safely be replaced by `string`)Commits-------0e590dc Allow scalar configuration in PHP Configuration
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.

4 participants
@HypeMC@nicolas-grekas@carsonbot@jderusse
[8]ページ先頭
©2009-2025 Movatter.jp