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

bug fix: take into account levels order #52

Closed
wants to merge 1 commit into from
Closed

Conversation

aliosv
Copy link

@aliosv aliosv commented Mar 18, 2015

No description provided.

@blond
Copy link
Member

blond commented Mar 18, 2015

Hi! What problem fixes this PR and how to reproduce it?

@tadatuta
Copy link
Member

#47

@aliosv
Copy link
Author

aliosv commented Mar 18, 2015

@andrewblond, при наличии в проекте css и styl не учитывается порядок уровней переопределения, технология клеит файлы по суффиксам(расширениям): сначала css, затем styl.
я заменил отсортированный список файлов по суффиксам на список всех файлов, где учтен порядок у.п. и отфильтровал нужные суффиксы вручную.

@blond
Copy link
Member

blond commented Mar 30, 2015

@aliosv, извини за долгий ответ.

Принять PR смогу только после того как добавлю тесты #36 (в процессе).

.builder(function (sourceFiles) {
var node = this.node,
filename = node.resolvePath(path.basename(this._target)),
defer = vow.defer(),
css, renderer;

css = sourceFiles.map(function (file) {
css = sourceFiles.items.filter(function (item) {
return ['css', 'styl'].indexOf(item.suffix) > -1;
Copy link
Member

Choose a reason for hiding this comment

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

Этот массив нужно брать из опции sourceSuffixes.

@aliosv
Copy link
Author

aliosv commented Apr 4, 2015

вынес суффиксы в опцию

@@ -29,14 +30,17 @@ module.exports = require('enb/lib/build-flow').create()
.defineOption('compress', false)
.defineOption('prefix', '')
.defineOption('variables')
.useFileList(['css', 'styl'])
.defineOption('source', ['css', 'styl'])
Copy link
Member

Choose a reason for hiding this comment

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

Чтобы не сломать обратную совместимость и ради консистентности среди других технологий, нужно назвать sourceSuffixes: https://github.com/enb-make/enb/blob/master/lib/build-flow.js#L133.

Copy link
Author

Choose a reason for hiding this comment

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

done

@blond
Copy link
Member

blond commented Jun 30, 2015

Выяснили, что проблема в ядре — enb/enb#232 и базовых технологиях — enb/enb-bem-techs#129.

Закрываю Pull Request, проблема решится, когда будут исправлены проблемы в ядре.

@blond blond closed this Jun 30, 2015
@blond blond removed the ready label Jun 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants