#11 Add tests for i18n-lang-js technology

[tormozz48]

Please review it

@blond @sipayRT

[sipayRT]

может лучше это добавить в свой глобальный .gitignore?

[blond]

Да, такое нужно уносить в свой ~/.gitignore.

[sipayRT]

пакеты ниже можно сразу обновить

[blond]

Нужно добавить тест на случай, когда файл с кейсетами пустой.

[tormozz48]

"Нужно добавить тест на случай, когда файл с кейсетами пустой."

1. А такой кейс реальный?
2. Пустой - это совсем пустой или module.exports = {} ?
3. Что мы должны получить в таком случае?

@blond

[blond]

А такой кейс реальный?

Да

Пустой - это совсем пустой или module.exports = {} ?

module.exports = {};

Что мы должны получить в таком случае?

Что-то в таком духе:

if (typeof BEM !== \'undefined\' && BEM.I18N) {
    BEM.I18N.lang(\'' + lang + '\');
}

[tormozz48]

Я исправил все замечания по этому pr.
Требуется вердикт

[sipayRT]

ну если тебе не по душе унести схему, то у меня других заметок нет. Кстати, посмотри, плз, мой комментарий - ты на него не ответил.

[coveralls]

Changes Unknown when pulling ce8c692 on #11 into * on master*.

[tadatuta]

кмк, пустой-пустой тоже может иметь смысл проверять, например, если файл с переводами закомментировали:

// module.exports = {
//  
// }

[sipayRT]

у тебя в двух тестах этот массив отличается только одним айтемом - ' "key": \'val2\''. Его можно перенести в buildWithCache, а в функцию уже передавать параметром expected что ты ожидаешь получить - 'val1' или 'val2'. Мне кажется, что станет красивее :) (имхо)

[tormozz48]

Сделал как ты сказал

[tormozz48]

"кмк, пустой-пустой тоже может иметь смысл проверять, например, если файл с переводами закомментировали:" - этот файл получается в результате работы предыдущей технологии i18n-merge-keysets.js и является автогенеренным. Сложно представить что его могули закомментировать

[coveralls]

Changes Unknown when pulling aa86f8d on #11 into * on master*.

[coveralls]

Changes Unknown when pulling d6ba8aa on #11 into * on master*.

[sipayRT]

@tormozz48 во, вот последний коммит прям красивый :)
Вот такое же я бы сделал и для expected как-то так:

expected = [
    'if (typeof BEM !== \'undefined\' && BEM.I18N) {BEM.I18N.decl(\'scope\', {',
    '    "key": ' + (initial.mtime === modified.mtime) ? initial.val : modified.val,
    '}, {',
    '"lang": "lang"',
    '});',
    '',
    'BEM.I18N.lang(\'lang\');',
    '',
    '}',
    ''
].join('\n');

но если считаешь, что это нечитабельно, то можно забить

[sipayRT]

в общем, у меня всё :)

[tormozz48]

@sipayRT
Насчет expected есть 2 возражения:

1. Во-первых он и так с трудом читаемый
2. Любые дополнительные абстракции усложняют понимание

[sipayRT]

@tormozz48 вот я об этом и говорю - не понятно что в этом большом куске кода должно поменяться. А переменные хоть подсветятся :) Но я не настаиваю, можно и так оставить

[coveralls]

Changes Unknown when pulling 132e6af on #11 into * on master*.