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

added clean option to assets install command #24216

Merged
merged 1 commit into from Dec 7, 2017

Conversation

Projects
None yet
7 participants
@robinlehrmann
Contributor

robinlehrmann commented Sep 15, 2017

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #24204, #23416
License MIT
@iltar

This comment has been minimized.

Contributor

iltar commented Sep 15, 2017

Should probably go into 3.4 as it's a new feature, even though I know why it "broke".

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Sep 16, 2017

@iltar this tries to remove a behavior change introduced in #23195

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Sep 16, 2017

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Sep 16, 2017

So, ping @stof, original reporter of #23177.

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Oct 12, 2017

I'd propose to reverse the flag, and submit it on 3.4, as a new feature, so that people can opt-out if they want (yes, that'd mean keeping the BC break, I know, let's live with that.)

@robinlehrmann robinlehrmann changed the base branch from 2.7 to master Nov 24, 2017

@xabbuh xabbuh added Feature and removed Bug labels Nov 24, 2017

@xabbuh xabbuh modified the milestones: 2.7, 4.1 Nov 24, 2017

@@ -58,6 +58,7 @@ protected function configure()
))
->addOption('symlink', null, InputOption::VALUE_NONE, 'Symlinks the assets instead of copying it')
->addOption('relative', null, InputOption::VALUE_NONE, 'Make relative symlinks')
->addOption('clean', null, InputOption::VALUE_NONE, 'Remove the assets of the bundles that no longer exist')

This comment has been minimized.

@xabbuh

xabbuh Nov 26, 2017

Member

@nicolas-grekas suggested the opposite (i.e. having a no-cleanup option or something similar) which would need to be passed if you do not want to have old assets removed.

@robinlehrmann

This comment has been minimized.

Contributor

robinlehrmann commented Nov 26, 2017

@xabbuh Okay, I changed it. Good point 👍

@xabbuh

xabbuh approved these changes Nov 26, 2017

@Tobion

Tobion approved these changes Nov 29, 2017

@fabpot

This comment has been minimized.

Member

fabpot commented Dec 7, 2017

Thank you @robinlehrmann.

@fabpot fabpot merged commit 771f11b into symfony:master Dec 7, 2017

3 checks passed

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

fabpot added a commit that referenced this pull request Dec 7, 2017

feature #24216 added clean option to assets install command (robinleh…
…rmann)

This PR was merged into the 4.1-dev branch.

Discussion
----------

added clean option to assets install command

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24204, #23416
| License       | MIT

Commits
-------

771f11b added clean option to assets install command

@robinlehrmann robinlehrmann deleted the robinlehrmann:added-clean-option-assets-install branch Dec 7, 2017

@fabpot fabpot referenced this pull request May 7, 2018

Merged

Release v4.1.0-BETA1 #27181

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