Skip to content

Conversation

@ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Nov 19, 2017

Q A
Branch? 3.4 / 4.1?
Bug fix? yes?
New feature? no
BC breaks? no?
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

we still run assets:install by default, which in bundle-less apps gives this annoying empty public/bundles dir and some useless cli output, all the time. This fixes it.


// Create the bundles directory otherwise symlink will fail.
$bundlesDir = $targetArg.'/bundles/';
$this->filesystem->mkdir($bundlesDir, 0777);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

given symlink() creates the parent directory, and mkdir() handles parents recursively, we could drop this line.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

ok for 3.4 to me

@fabpot
Copy link
Member

fabpot commented Nov 20, 2017

Thank you @ro0NL.

@fabpot fabpot merged commit f8e7478 into symfony:3.4 Nov 20, 2017
fabpot added a commit that referenced this pull request Nov 20, 2017
…efault (ro0NL)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Dont create empty bundles directory by default

| Q             | A
| ------------- | ---
| Branch?       | 3.4 / 4.1?
| Bug fix?      | yes?
| New feature?  | no
| BC breaks?    | no?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->

we still run `assets:install` by default, which in bundle-less apps gives this annoying empty public/bundles dir and some useless cli output, all the time. This fixes it.

Commits
-------

f8e7478 [FrameworkBundle] Dont create empty bundles directory
@ro0NL ro0NL deleted the assets-install branch November 20, 2017 07:49
This was referenced Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants