Add 'update' command

[sipayRT]

// cc @j0tunn @SwinX @leonsabr

[SwinX]

Метод не использует ничего, что предоставляет инстанс класса. Нужно сделать статическим.

[j0tunn]

Тогда вместо this.getStateData придется писать что-то типа this.__self.getStateData

[SwinX]

почему? Сделать его методом класса и писать CaptureProcessor._getStateData(). Что за self?

[j0tunn]

ты будешь звать его в наследниках, а там хз от кого ты отнаследовался, все время подключать capture-processor - не айс. __self - это ссылка на класс в инстансе

[SwinX]

Начало про refPath и окончание про резолв промиса с какими-то данными одинаково для new-updaterа и diff-updaterа. Нужно бы подумать, как это вынести в базовый класс.

Идея такая: базовый screen-updater делает какой-то warm-up вроде вычисление refPath (может что-то ещё, короче всё общее), отдает эти данные в protected-метод, например _doProcessCapture и в конце цепляется на вернувшийся из _doProcessCapture промис, завершая цепочку отдачей данных.

Как видится реализация:

processCapture: function(capture) {
    var _this = this;
    this._prepareToProcessCapture()
        .then(function(prepared) {
            return _this._doProcessCapture(prepared, capture);
        })
        .thenResolve(this._getStateData(prepared.path, capture));
}

Ещё 1 профит от такого рефакторинга - эти данные не будут считаться 2 раза в meta-updaterе

[SwinX]

Кстати, а зачем вообще остался screen-shooter? В теории всё, что он раньше делал, теперь должен делать new-updater.

[sipayRT]

Кстати, а зачем вообще остался screen-shooter? В теории всё, что он раньше делал, теперь должен делать new-updater

только потому что буду его выпиливать отдельным коммитом. Изначально я надеялся сделать это все до v2.0.0, поэтому нарочно его не трогал

[SwinX]

< captain mode > нужно покрыть тестами</ captain mode >

[j0tunn]

else не нужен. Ну и в принципе этот if тоже не нужен

[sipayRT]

если if вообще убрать, то получится такая ситуация, при которой я напишу любую опцию и мне вернется NewUpdater. Типа, gemini update --some запустится как с опцией --new. Мы хотим такого поведения?

[j0tunn]

ну вообще, это бы на уровне commander'а сделать, типа Unknown option

[sipayRT]

как это правильно сделать? у нас сейчас в commander'е включена опция allowUnknownOption, чтобы можно было передавать опции для gemini дальше. отключить мы ее не можем.

[j0tunn]

ну тогда:

if (opts.diff && !opts.new) {
    return DiffUpdater
}

if (!opts.diff && opts.new) {
    return new NewUpdater
}

return MetaUpdater

и хоть трава не расти

[j0tunn]

Идея такая: базовый screen-updater делает какой-то warm-up вроде вычисление refPath (может что-то ещё, короче всё общее), отдает эти данные в protected-метод, например _doProcessCapture и в конце цепляется на вернувшийся из _doProcessCapture промис, завершая цепочку отдачей данных.

Это паттерн template method. И да, он сюда просится

[j0tunn]

а не будет проблем, что это вызывается в двух местах?

gemini/lib/capture-processor/tester.js

Line 14 in bfc0e75

temp.track();

[sipayRT]

он только трекает все изменения темповой директории. Ну и тестер не будет вызываться, когда используется update

[j0tunn]

нехорошо использовать чужие private/protected методы. MetaUpdater можно унаследовать от CaptureProcessor и переопределить processCapture

[j0tunn]

Хотя блин, тогда вся общая логика будет повторяться два раза

[j0tunn]

ладно, пускай будет как есть

[j0tunn]

На meta-updater можно написать только те тесты, которых нет в двух других - типа что там все не запускается по два раза. Ну и еще несколько комплексных: типа 'если есть diff и есть тест без эталона', или 'есть diff, но нет теста без эталона', 'нет diff'а, но есть тест без эталона', 'нет нихрена'

[j0tunn]

не зачем, лучше сразу в тестах писать ImageProcessor.prototype.blabla.returns... тогда сразу понятно, что любой инстанс вернет такое значение

[sipayRT]

я ее создал, чтобы мне было удобней и короче писать :(

[j0tunn]

короче - не всегда понятнее
Если ты видешь в коде теста:

ImageProcessor.prototype.blaBla.returns(true);

то сразу становится понятно, что любой инстанс ImageProcessor'а при вызове blaBla вернет true

А если видишь

imageProcessor.blaBla.returns(true);

то нужно узнать, что это за imageProcessor такой (причем создается впечатление, что как раз и есть какой-то конкретный инстанс), где он создается, кем возвращается