-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 template minification to keep line breaks, remove html files from bindata #5023
Conversation
fyi @sg00dwin |
htmlmin: '<%= htmlmin.dist.options %>', | ||
usemin: 'scripts/scripts.js' | ||
usemin: 'scripts/templates.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jwforres if you take out the usemin
option, does it still minify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking if you set dest
to the dist
directory and take out usemin
do you need the extra copy task below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yeah we don't need usemin anymore. Since there will always be a file called scripts/templates.js we can just harcode it into index.html like we do with config.js
9204dd7
to
06b1041
Compare
@spadgett updated to take out usemin and copy bits that weren't needed. Double checked both dev and built modes and they were working as expected. |
LGTM, [merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3557/) (Image: devenv-fedora_2455) |
Evaluated for origin merge up to 06b1041 |
[Test]ing while waiting on the merge queue |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/5610/) |
Evaluated for origin test up to 06b1041 |
Merged by openshift-bot
Fixes #4582