Распаковка расширений на исходники #106

JohnyDeath · 2017-02-09T20:03:19Z

Добавлена возможность подхвата и распаковки расширений на исходники

artbear

Хорошое дополнение, только нужны небольшие доработки.

artbear · 2017-02-12T14:57:03Z

v8files-extractor.os

+	Конфигуратор.ВыполнитьКоманду(Параметры);
+	Лог.Отладка("Вывод 1С:Предприятия - " + Конфигуратор.ВыводКоманды());
+
+	Лог.Отладка(СтрШаблон("Разбор расширения '%1' в исходники в каталог '%2'", ИмяРасширения, ПапкаИсходников.ПолноеИмя));


В Лог.Отладка (и других командах вывода логирования) давно уже можно стандартные параметры %1 и т.п.
Т.е совсем не нужен доп.вызов СтрШаблон, можно просто Лог.Отладка

В этом скрипте вообще много разных способов подстановки параметров в строку увидел. Надо б тоже порефакторить

Давай точечно, исправь свой код + немного уже существующего, а потом еще доработаем.

artbear · 2017-02-12T14:58:01Z

v8files-extractor.os

+	Лог.Отладка("Вывод 1С:Предприятия - " + Конфигуратор.ВыводКоманды());
+
+	Лог.Отладка(СтрШаблон("Разбор расширения '%1' в исходники в каталог '%2'", ИмяРасширения, ПапкаИсходников.ПолноеИмя));
+	Параметры = Конфигуратор.ПолучитьПараметрыЗапуска();


Параметры = Конфигуратор.ПолучитьПараметрыЗапуска();
Здесь случайно не учтутся предыдущие параметры Конфигуратора, установленные ранее?

Нет. Всегда возвращается новый массив параметров. Собственно для этого он здесь и вызывается.

artbear · 2017-02-12T14:59:23Z

v8files-extractor.os

+	Конфигуратор.ВыполнитьКоманду(Параметры);
+	Лог.Отладка("Вывод 1С:Предприятия - " + Конфигуратор.ВыводКоманды());
+
+	Лог.Отладка("Очищаем каталог временной ИБ");


В конце метода написано Лог.Отладка("Очищаем каталог временной ИБ");
но никакого кода по очистке нет.
Почему так?

В начале модуля объявляется: КаталогВременнойИБ = ВременныеФайлы.СоздатьКаталог();
Как я понимаю, этот класс подчищает временные файлы после удаления его экземпляра. Или я не прав?

Не прав. Никто ничего не очищает

@EvilBeaver , конечно, деструктора ж нет ;)
@artbear вот тут аналогично: https://github.com/xDrivenDevelopment/precommit1c/blob/develop/v8files-extractor.os#L312
Я так понимаю, мой PR плавно перетекает в рефакторинг всего скрипта? )

Вроде же в конце скрипта идёт удаление временных файлов, чего вы набежали)

нет, полный рефакторинг не нужен.
Мне не нравится "обманывающее" сообщение.
Можно:

убрать это сообщение из указанного места

и вставить его перед реальным удалением временных файлов

artbear · 2017-02-12T15:02:25Z

v8files-extractor.os

+	КаталогВременнойИБ = ВременныеФайлы.СоздатьКаталог();
+	Конфигуратор.КаталогСборки(КаталогВременнойИБ);
+
+	ЛогКонфигуратора = Логирование.ПолучитьЛог("oscript.lib.v8runner");


Предлагаю код

ЛогКонфигуратора = Логирование.ПолучитьЛог("oscript.lib.v8runner"); + ЛогКонфигуратора.УстановитьУровень(Лог.Уровень()); + ЛогКонфигуратора.Закрыть();

отрефакторить, выделив единый метод УстановитьУровеньЛогаКонфигуратораРавнымУровнюПродукта или аналогичное название.
Иначе опять куча дублирующего кода.

Согласен. Но тогда придется порефакторить весь скрипт.

Стойте! Не надо рулить логами вообще! В логос теперь есть внешнее управление уровнями, а программно этого делать больше не надо.

@EvilBeaver т.е. теперь строка ЛогКонфигуратора.УстановитьУровень(Лог.Уровень());
вообще не нужна?

Пока не реализовано "новое" поведение, нельзя выкидывать старое поведение.

Под "не реализовано новое поведение" я понимаю, что если мы просто уберем указанный код, то уберем появление полезных сообщений, что важно для CI, например.
Breaking change нужно аккуратно вводить.

JohnyDeath · 2017-02-13T19:38:03Z

Прошелся по всем СтрШаблон из Лога, а также поправил код по замечаниям

artbear · 2017-02-14T06:13:39Z

v8files-extractor.os

 Функция Версия() Экспорт

-	Версия = "2.1.0-PRE";
+	Версия = "2.2.0-PRE";


Предлагаю использовать нормальную версию 2.2, без всяких PRE

nixel2007 · 2017-02-14T07:02:17Z

Вот не надо было версию менять, она же не просто так была установлена. Мало того, релиза 2.1 еще не было, последний релиз - 2.0.6. А теперь 2.2 впаяли :)

JohnyDeath · 2017-02-14T10:26:39Z

@nixel2007 Мои мысли такие:
Здесь висела версия 2.1.0
Добавили новый функционал - поменяли предпоследнюю цифру.
Или расскажите какая политика версий должна быть

nixel2007 · 2017-02-14T10:28:25Z

@JohnyDeath уже не помню кто просил меня менять версию в девелопе на пре-релиз, чтобы отличать с какой ветки люди качали прекоммит.
мне еще тогда эта практика не нравилась, а теперь, когда есть опм, то и подавно надо от нее отказаться.

JohnyDeath · 2017-02-14T10:40:16Z

@nixel2007 так в итоге какую я должен был версию ставить? Или вообще не трогать?

nixel2007 · 2017-02-14T12:03:48Z

Не трогать. Но это не важно, выпилим эту схему вообще в скоре. 2017-02-14 13:40 GMT+03:00 Evgeny Martynenkov <notifications@github.com>:

…

@nixel2007 <https://github.com/nixel2007> так в итоге какую я должен был версию ставить? Или вообще не трогать? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#106 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABFJKL8XQMrCysOc1zKkjp6LMdOJcm-cks5rcYSQgaJpZM4L8mic> .

JohnyDeath added 2 commits February 9, 2017 22:50

Распаковка расширений на исходники

7d57dd7

Добавлено упоминание расширений в ридми

6a4d5bd

artbear requested changes Feb 12, 2017

View reviewed changes

artbear requested review from pumbaEO and nixel2007 February 12, 2017 15:03

Рефакторинг и замечания PR

549b337

artbear reviewed Feb 14, 2017

View reviewed changes

Исправил версию на 2.2.0

fa0786d

artbear merged commit 2586075 into xDrivenDevelopment:develop Feb 14, 2017

artbear added enhancement oscript-port labels Feb 14, 2017

artbear removed request for pumbaEO and nixel2007 February 14, 2017 06:18

artbear modified the milestone: 2.2 Feb 14, 2017

Распаковка расширений на исходники #106

Распаковка расширений на исходники #106

Uh oh!

Conversation

JohnyDeath commented Feb 9, 2017

Uh oh!

artbear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnyDeath commented Feb 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nixel2007 commented Feb 14, 2017

Uh oh!

JohnyDeath commented Feb 14, 2017

Uh oh!

nixel2007 commented Feb 14, 2017

Uh oh!

JohnyDeath commented Feb 14, 2017

Uh oh!

nixel2007 commented Feb 14, 2017 via email

Uh oh!

Uh oh!