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.

Sign up for GitHub

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

Jump to bottom

Add normalize2 stub and tests #41

Merged
merged 2 commits into from
Jul 26, 2016
Merged

Add normalize2 stub and tests #41

merged 2 commits into from
Jul 26, 2016

Conversation

skad0
Copy link
Member

@skad0 skad0 commented Jul 2, 2016

Test to understand and format required format

blond
blond reviewed Jul 2, 2016
View reviewed changes
test/normalize2/block.test.js
const test = require('ava');
const normalize = require('../../lib/normalize2');

test('should support block', t => {
Copy link
Member

Choose a reason for hiding this comment

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

Нужен кейс про массив из блоков.

Copy link
Contributor

Choose a reason for hiding this comment

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

Добавили

@blond
Copy link
Member

blond commented Jul 2, 2016

Хорошо бы подумать про обработку невалидных кейсов, которые, не указаны в SchemeJSON.

@skad0 skad0 self-assigned this Jul 3, 2016
qfox
qfox reviewed Jul 19, 2016
View reviewed changes
lib/normalize2.js
}

if (!isNotActual(mod) && !elem) {
normalizeMods(block, mod);
Copy link
Contributor

Choose a reason for hiding this comment

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

почему не сделать {block, mod} ?

@qfox
Copy link
Contributor

qfox commented Jul 20, 2016

bemDecl.normalizer('normalize2')({elem: 'control'}) выдает [ { block: undefined, elem: 'q' } ]
Это прекрасно, но не очень.

Теоретически можно после нормализации делать .map(item => fulfill(item, scope))
Или даже .map(fulfill.bind(this, scope)), надо будет только в fulfill скоуп поставить на первое место.

Даже если оно сейчас будет работать после fulfill правильно — block: undefined выглядит не очень понятно.

@qfox qfox force-pushed the issue3 branch 4 times, most recently from f328972 to 9676535 Compare July 26, 2016 02:33
@qfox
Copy link
Contributor

qfox commented Jul 26, 2016

Хорошо бы подумать про обработку невалидных кейсов, которые, не указаны в SchemeJSON.

#45

@qfox
Copy link
Contributor

qfox commented Jul 26, 2016

@Yeti-or @tadatuta @skad0 Я готов вливать ;-) Всё норм?

@skad0
Copy link
Member Author

skad0 commented Jul 26, 2016

@zxqfox да, проблем не вижу, спасибо

@tadatuta
Copy link
Member

lgtm

Yeti-or
Yeti-or reviewed Jul 26, 2016
View reviewed changes
lib/normalize2.js

if (!decl) { return res; }

if (typeof decl === 'string' || !(Symbol.iterator in decl)) {
Copy link
Member

Choose a reason for hiding this comment

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

!(Symbol.iterator in decl) это лучше чем Array.isArray? или предпологаем что могут быть любые итерируемые?

Copy link
Contributor

Choose a reason for hiding this comment

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

nullable
https://developers.google.com/closure/compiler/docs/js-for-compiler

@Yeti-or
Copy link
Member

Yeti-or commented Jul 26, 2016

🎱

@qfox qfox merged commit d9dea23 into master Jul 26, 2016
@qfox qfox deleted the issue3 branch July 26, 2016 13:53
@skad0 skad0 mentioned this pull request Feb 2, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@skad0 skad0

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@skad0 @blond @qfox @tadatuta @Yeti-or