Skip to content
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

fix(@angular-devkit/build-angular): change css optimizer from clean-css with cssnano #16539

Merged
merged 2 commits into from
Jan 13, 2020
Merged

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Jan 3, 2020

Closes #16123 and closes #13854

@alan-agius4 alan-agius4 added the needs: discussion On the agenda for team meeting to determine next steps label Jan 3, 2020
@alan-agius4 alan-agius4 changed the title wip! use cssnano fix(@angular-devkit/build-angular): change css optimizer from clean-css with cssnano Jan 4, 2020
@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release and removed needs: discussion On the agenda for team meeting to determine next steps labels Jan 4, 2020
@alan-agius4 alan-agius4 marked this pull request as ready for review January 4, 2020 14:09
@filipesilva
Copy link
Contributor

I think this should target master only though, there's risk of changes in behaviour and there isn't a big need to couple that with 9.0.

@alan-agius4 alan-agius4 added the needs: discussion On the agenda for team meeting to determine next steps label Jan 6, 2020
@alan-agius4 alan-agius4 requested review from clydin and removed request for clydin January 8, 2020 13:58
@alan-agius4 alan-agius4 removed the target: patch This PR is targeted for the next patch release label Jan 8, 2020
@IgorMinar
Copy link
Contributor

if we are deciding between making this change in 9.0 vs 9.1 then let's do it in 9.0. If we think the risk of breaking changes is high, then we should not make this change in 9.x at all and wait for v10.

The reasoning here is that this PR fixes features enabled by v9.0 framework changes so it would be good to get them to 9.0 as a fix, but if the risk of making this change is high then we should not make it in 9.x at all.

@alan-agius4 I'll let you assess which one is right given the guidelines above.

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release and removed needs: discussion On the agenda for team meeting to determine next steps labels Jan 10, 2020
@alan-agius4
Copy link
Collaborator Author

Unless @clydin or @filipesilva feel otherwise, I think we should go ahead and put it in version 9 as opposed to version 10. While there is a risk of breakages, I think it is manageable, worst case we can land a trivial fix to disable the rule that causes the problem.

@alan-agius4 alan-agius4 added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Jan 11, 2020
@alan-agius4
Copy link
Collaborator Author

Patch version: #16647

@alan-agius4 alan-agius4 deleted the css-nano branch January 13, 2020 19:37
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS Variables are lost when running ng build --prod CleanCss breaks :host-context(selector selector)
6 participants