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] remove dead conditions in Translation Commands#42362

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

Closed
acran wants to merge0 commits intosymfony:5.4fromacran:5.3

Conversation

@acran
Copy link
Contributor

@acranacran commentedAug 3, 2021
edited by Nyholm
Loading

QA
Branch?5.3
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

This is just a trivial removal of unused code I stumbled upon while debugging#42361. In theoriginal code:

$transPaths = [$path.'/translations'];$codePaths = [$path.'/templates'];if (!is_dir($transPaths[0]) && !isset($transPaths[1])) {thrownewInvalidArgumentException(sprintf('"%s" is neither an enabled bundle nor a directory.',$transPaths[0]));}

The second part of the conditionisset($transPaths[1]) willalways evaluate to true, since$targetPath is just set 3 lines above but only has a single element.

This check was originally to support legacy paths which was removed inb6eb1f4:

Copy link
Member

@NyholmNyholm left a comment

Choose a reason for hiding this comment

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

Thank you.

I agree with this change. I updated the pull request description so it is using the template. (Fabbot is happy now).

Im not sure if we should target an earlier branch or not.

acran reacted with thumbs up emoji
@acran
Copy link
ContributorAuthor

@Nyholm 5.3 is the oldest still supported branch containing this code;4.4 still has the legacy paths supported

Nyholm reacted with thumbs up emoji

@Nyholm
Copy link
Member

Cool. Thank you for checking for me.

@fabpot
Copy link
Member

As this is not a bug fix, I will merge this PR in 5.4.

@fabpot
Copy link
Member

Something went very wrong here.@acran Can you push your code again? Sorry for that.

@acran
Copy link
ContributorAuthor

@fabpot do you mean in a new PR on top of 5.4? Or into this PR again?

I pushed it intomy fork again. But ... did you somehow manage to force-push into my fork of the repo without access? o0

@fabpot
Copy link
Member

@acran I think you will need to create a new PR on 5.4.

fabpot added a commit that referenced this pull requestAug 6, 2021
…Commands (acran)This PR was merged into the 5.4 branch.Discussion----------[FrameworkBundle] remove dead conditions in Translation Commands| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This is just a trivial removal of unused code I stumbled upon while debugging#42361. In the [original code](https://github.com/symfony/symfony/blob/e617a9b/src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.php#L165-L170):~~~php$transPaths = [$path.'/translations'];$codePaths = [$path.'/templates'];if (!is_dir($transPaths[0]) && !isset($transPaths[1])) {throw new InvalidArgumentException(sprintf('"%s" is neither an enabled bundle nor a directory.', $transPaths[0]));}~~~The second part of the condition `isset($transPaths[1])` will **always** evaluate to true, since `$targetPath` is just set 3 lines above but only has a single element.This check was originally to support legacy paths which was removed inb6eb1f4:* in [`TranslationDebugCommand.php`](b6eb1f4#diff-67afa5b8860d0df4e44f1e1fc89f444b7ac77de515b698a6824dd5403a0acdbcL187-L194)* in [`TranslationUpdateCommand.php `](b6eb1f4#diff-a01c7858e84f1868a427634740511da7c8c73e56772baa78bdcd98200d7125c0L180-L187)Rebased from 5.3 to 5.4, see#42362/cc `@fabpot`Commits-------22db5ad [FrameworkBundle] remove dead conditions in Translation Commands
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@NyholmNyholmNyholm approved these changes

@chalasrchalasrAwaiting requested review from chalasr

@wouterjwouterjAwaiting requested review from wouterj

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

4 participants

@acran@Nyholm@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp