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

[FrameworkBundle] Move console configuration to PHP#37216

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

Conversation

@AhmedRaafat14
Copy link

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets#37186
LicenseMIT
Doc PRn/a

@AhmedRaafat14AhmedRaafat14force-pushed theframworkbundle-rip-xml-console branch 2 times, most recently fromfbfb026 to1d88889CompareJune 11, 2020 14:24
Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

@AhmedRaafat14 you were fast! This looks ready to merge. Thanks!

AhmedRaafat14 reacted with thumbs up emojiAhmedRaafat14 reacted with hooray emoji
@AhmedRaafat14AhmedRaafat14force-pushed theframworkbundle-rip-xml-console branch fromc680793 to071c02aCompareJune 11, 2020 15:01
service('translation.extractor'),
param('translator.default_path'),
abstract_arg('twig.default_path'),
abstract_arg('Translator paths'),
Copy link
Member

@ycerutoycerutoJun 11, 2020
edited
Loading

Choose a reason for hiding this comment

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

<argument type="collection" />

Well, it seems we can't useabstract_arg() here, or we should add a newcollection_abstract_arg() or something else to set[] as default.

The tests can be solved by setting[] for both collection arguments.

AhmedRaafat14 reacted with thumbs up emoji
Copy link
Member

@ycerutoycerutoJun 11, 2020
edited
Loading

Choose a reason for hiding this comment

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

@AhmedRaafat14 I afraid we need to change it again to[], // Translator paths, sorry, same for Twig paths argument (same for both commands), thanks!

AhmedRaafat14 reacted with thumbs up emoji

Choose a reason for hiding this comment

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

I was I did that it fixes the tests then I thought it might be me mistaken.

Thank you for confirming that I will update the PR ASAP.

Copy link
Author

@AhmedRaafat14AhmedRaafat14Jun 11, 2020
edited
Loading

Choose a reason for hiding this comment

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

@javiereguiluz@yceruto
To fix the testing I had to revert back everycollection argument to an empty array[] with a comment.

Also even for theconsole.command.translation_debug the unit tests failed with usingabstract_arg('twig.default_path') it throws an exception that it expects string but got anAbstractArgument.
I will give it sometime tomorrow to try to fix it :) if I can. (in seperate PR for sure)

What do you think?

Exception Example:

Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeException: Invalid definition for service "console.command.translation_debug": argument 5 of "Symfony\Bundle\FrameworkBundle\Command\TranslationDebugCommand::__construct" accepts "string", "Symfony\Component\DependencyInjection\Argument\AbstractArgument" passed.

@AhmedRaafat14AhmedRaafat14force-pushed theframworkbundle-rip-xml-console branch 2 times, most recently from9b07f74 toef01839CompareJune 11, 2020 20:51
Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

AhmedRaafat14 reacted with thumbs up emoji
@fabpot
Copy link
Member

Thank you@AhmedRaafat14.

AhmedRaafat14 reacted with thumbs up emoji

fabpot added a commit that referenced this pull requestOct 26, 2021
…lt is disabled (GromNaN)This PR was merged into the 5.3 branch.Discussion----------[Framework][Secrets] Fix service definition when local vault is disabled| Q             | A| ------------- | ---| Branch?       | 5.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Local vault can be disabled with the following configuration:```framework:    secrets:        local_dotenv_file: ~```This removes the service `secrets.local_vault` ([code](https://github.com/symfony/symfony/blob/5.3/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L1669)). The `secrets:...` commands support this by having `$localVault` argument nullable.When configuration was moved from XML to PHP in#37216, the attribute `on-invalid="ignore"` of some services was left.Commits-------0e11f5a Fix commands when local vault is disabled
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoyceruto approved these changes

@javiereguiluzjaviereguiluzAwaiting requested review from javiereguiluz

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AhmedRaafat14@fabpot@javiereguiluz@yceruto@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp