8000 [DependencyInjection] inline conditional statements by hhamon · Pull Request #21785 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] inline conditional statements #21785

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 28, 2017
Merged

[DependencyInjection] inline conditional statements #21785

merged 1 commit into from
Feb 28, 2017

Conversation

hhamon
Copy link
Contributor
@hhamon hhamon commented Feb 27, 2017
Q A
Branch? 2.8
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR ~

@hhamon hhamon changed the base branch from master to 2.8 February 27, 2017 16:01
@hhamon hhamon changed the title [DependencyInjection] inline if statement [DependencyInjection] inline conditional statements Feb 27, 2017
@nicolas-grekas nicolas-grekas added this to the 2.8 milestone Feb 27, 2017
@nicolas-grekas
Copy link
Member

why not 2.7?

@nicolas-grekas nicolas-grekas modified the milestones: 2.7, 2.8 Feb 27, 2017
@fabpot
Copy link
Member
fabpot commented Feb 27, 2017

I don't see the point of such changes.

HeahDude
HeahDude reviewed Feb 27, 2017
View reviewed changes
src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -1029,11 +1029,7 @@ private function addMethodMap()
private function addAliases()
{
if (!$aliases = $this->container->getAliases()) {
if ($this->container->isFrozen()) {
return "\n \$this->aliases = array();\n";
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just remove the else?

@hhamon hhamon changed the base branch from 2.8 to 2.7 February 28, 2017 09:14
@nicolas-grekas nicolas-grekas merged commit b7b7c54 into symfony:2.7 Feb 28, 2017
nicolas-grekas added a commit that referenced this pull request Feb 28, 2017
@nicolas-grekas
minor #21785 [DependencyInjection] inline conditional statements (hha…
c1b3422 
…mon)

This PR was merged into the 2.7 branch.

Discussion
----------

[DependencyInjection] inline conditional statements

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | ~

<!--
- Bug fixes must be submitted against the lowest branch where they apply
  (lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
- Please fill in this template according to the PR you're about to submit.
- Replace this comment by a description of what your PR is solving.
-->

Commits
-------

b7b7c54 [DependencyInjection] inline conditional statements.
@nicolas-grekas
Copy link
Member

merged so that we don't spend more time on this (it will come up sometime later otherwise :) )
thanks @hhamon

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

@HeahDude HeahDude HeahDude left review comments

Assignees
No one assigned
Labels
DependencyInjection Status: Needs Review
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
5 participants
@hhamon @nicolas-grekas @fabpot @HeahDude @carsonbot
0