Closed nixel2007 closed 7 years ago
@pumbaEO @EvilBeaver Нас с Никитой все устраивает, код-ревью и проверки сделали. Если у вас нет вопросов, мержим и выпускаем релиз opm?
У меня есть замечание. В opm уже предусмотрен клиентский кастомизирущий скрипт. Данный PR делает второй "спецфайл", но для этапа сборки по образу и подобию первого.
Однако, для разработчика уже есть клиентский кастомизирующий скрипт. Это, внезапно, packagedef.
Я помню дискуссию о том "код" это или все-таки "конфиг". Когда я задумывал opm и воровал packagedef с Ruby, я своровал оттуда и идею кодоконфига. По задумке - "перехватчик сборки" это и есть packagedef.
В принципе, если есть конкретные аргументы, почему это должен быть отдельный файл, я буду рад их услышать и принять.
Как вариант, можно было бы добавить в файл КонстантыOpm.ИмяФайлаСкриптаУстановки
еще одну процедуру ПриСборке
, а не плодить еще один файл с "волшебным" именем. Хотя это и не бог весть, какая хорошая идея.
@EvilBeaver основная проблема использования packagedef - он зачитывается при различных операциях внутри opm - установки по зависимостям описания пакета, установки самого пакета, сборки и прочих.
Это приводит к тому, что для vrunner, например, шаг сборки обработок (который добавлялся в packageDef для команды build) начал выполняться и при обычном opm install
.
Вероятно частично могло бы помочь пробрасывание текущего режима (install/build/etc...) в контекст описания пакета, но и усложнения того же packagedef это бы добавило.
Предлагаемая реализация, имхо, наиболее компромисная со строгим разделением логики и ответственности между файлами.
При opm install он зачитывается для генерации xml которую съедает потом vsc?
Нет, из него зачитываются зависимости пакета и вызывается рекурсивное их разрешение/установка.
Ну окай, я так-то не против.
А мне нравится возвращение к идее кодоконфига.
Может быть, прямо в packadef и определять спец.методы обработчики событий, аналогично build.os и install.os ?
Это вроде бы несложно сотворить.
Link: https://github.com/silverbulleters/vanessa-runner/issues/135