Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

feat(build): Ignore compiled sass/less files from git #1592

Merged
merged 3 commits into from
Oct 29, 2016

Conversation

shanavas786
Copy link
Contributor

Compiling sass/less files to <module>/client/css directory makes it difficult to differentiate between compiled and raw css files.

By compiling them to appropriate directories, compiled files can be easily ignored.

@@ -31,7 +31,7 @@ module.exports = {
tests: ['public/lib/angular-mocks/angular-mocks.js']
},
css: [
'modules/*/client/css/*.css'
'modules/*/client/{css,less,scss}/*.css'
Copy link
Member

Choose a reason for hiding this comment

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

How does this affect the less & sass settings below?

@@ -169,9 +169,6 @@ gulp.task('sass', function () {
return gulp.src(defaultAssets.client.sass)
.pipe(plugins.sass())
.pipe(plugins.autoprefixer())
.pipe(plugins.rename(function (file) {
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed?

@shanavas786
Copy link
Contributor Author

@mleanos Now, both less and sass files are compiled to module/client/css/ directory [1], [2],
and only css files in module/client/css/ directory is used as css assets [3].

My proposal is to compile sass/less files to module/client/(sass/less) directory itself instead of css directory (that is why directory rename is removed) and include css files from those directories also in css assets (add scss/less directories to css asset path).

@mleanos
Copy link
Member

mleanos commented Oct 25, 2016

@shanavas786 Thank you. That perfectly answered my questions. Also, I agree with the goals of these changes. It's definitely gonna be easier to manage these compiled assets.

@meanjs/contributors comments?

Copy link
Member

@mleanos mleanos left a comment

Choose a reason for hiding this comment

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

LGTM. I haven't tested though, but the changes seem pretty straight forward.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants