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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DependencyInjection] add $lazyLoad context to the generated code for lazy non-shared service by PhpDumper #30093

Merged
merged 1 commit into from Feb 8, 2019

Conversation

Projects
None yet
3 participants
@XuruDragon
Copy link
Contributor

XuruDragon commented Feb 6, 2019

Q A
Branch? 4.1
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29930
License MIT
Doc PR n/a

Fix #29930 by adding $lazyLoad context to the generated code for lazy non-shared service by PhpDumper

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 6, 2019

Nice. Could you please add a test case with a fixture in PhpDumperTest.php please (see calls to assertStringEqualsFile for inspiration)

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Feb 6, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 6, 2019

Please rebase for 4.2 also as 4.1 is EOLed.

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

XuruDragon commented Feb 6, 2019

Nice. Could you please add a test case with a fixture in PhpDumperTest.php please (see calls to assertStringEqualsFile for inspiration)

I'll add it

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29930 branch 3 times, most recently from 37917d9 to 5c84edf Feb 6, 2019

@XuruDragon XuruDragon changed the base branch from 4.1 to 4.2 Feb 7, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29930 branch 5 times, most recently from 98575cc to a16f92d Feb 7, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 7, 2019

Another tweak: in PhpDumper on L690:
$factoryCode = $asFile ? ($definition->isShared() ? "\$this->load('%s.php', false)" : "\$this->factories['%s'](false)") : '$this->%s(false)';

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29930 branch 2 times, most recently from c156d03 to df8dd1d Feb 7, 2019

[DependencyInjection] fix #29930 add $lazyLoad flag to the generated …
…factory code for lazy non-shared services

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29930
| License       | MIT
| Doc PR        | n/a

Fix #29930  by adding $lazyLoad context to the generated code for lazy non-shared service by PhpDumper

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29930 branch from df8dd1d to 98d4dfd Feb 7, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 8, 2019

Thank you @XuruDragon.

@nicolas-grekas nicolas-grekas merged commit 98d4dfd into symfony:4.2 Feb 8, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Feb 8, 2019

bug #30093 [DependencyInjection] add $lazyLoad context to the generat…
…ed code for lazy non-shared service by PhpDumper (XuruDragon)

This PR was merged into the 4.2 branch.

Discussion
----------

[DependencyInjection] add $lazyLoad context to the generated code for lazy non-shared service by PhpDumper

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29930
| License       | MIT
| Doc PR        | n/a

Fix #29930  by adding $lazyLoad context to the generated code for lazy non-shared service by PhpDumper

Commits
-------

98d4dfd [DependencyInjection] fix #29930 add $lazyLoad flag to the generated factory code for lazy non-shared services

@XuruDragon XuruDragon deleted the XuruDragon:issue-29930 branch Feb 8, 2019

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