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

[Translation][FrameworkBundle] Add failing test for BC break regarding translation resources.#25079

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
jenkoian wants to merge1 commit intosymfony:2.7fromjenkoian:translator-resources

Conversation

@jenkoian
Copy link
Contributor

@jenkoianjenkoian commentedNov 21, 2017•
edited
Loading

QA
Branch?2.7
Bug fix?yes
New feature?no
BC breaks?yes (highlights a BC break)
Deprecations?no
Tests pass?no (on purpose in this case)
Fixed ticketsN/A (but see#23057 (comment))
LicenseMIT
Doc PRN/A

Adding a failing test case to prove a BC break caused by#23057

->expects($this->any())
->method('load')
->will($this->returnValue($this->getCatalogue('fr',array(
'foo' =>'foo (fr)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be inlined I think.

jenkoian reacted with thumbs up emoji
->expects($this->any())
->method('load')
->will($this->returnValue($this->getCatalogue('fr',array(
'bar' =>'bar (fr)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be inlined I think.

jenkoian reacted with thumbs up emoji
@nicolas-grekas
Copy link
Member

@jenkoian would you like to work on a fix? anyone else who would like to investigate?

@jenkoian
Copy link
ContributorAuthor

jenkoian commentedJan 11, 2018•
edited
Loading

@nicolas-grekas yeah I can have a go at a fix, before I waste any time though, is it agreed this is a bug? Expected behaviour is that loaders added after init should work the same as loaders added before init?

@nicolas-grekas
Copy link
Member

any hint from anyone?@yceruto maybe?

@jenkoian
Copy link
ContributorAuthor

See@stof's comments on#23057 (comment)

Creating a circular reference between the translator and the loader to have the loader reconfiguring the translator regarding what resources should be loaded by the loader is indeed a use case we never intended to support.

So likely, I can just close this PR? Although there are instances this could manifest outside a circular reference I guess, like the test case.

@nicolas-grekas
Copy link
Member

OK, I'm closing then.@stof if you think otherwise, please report here :)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@hhamonhhamonhhamon left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

2.7

Development

Successfully merging this pull request may close these issues.

4 participants

@jenkoian@nicolas-grekas@hhamon@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp