Created attachment 14395 [details] ssh_public_key Псевдоним (имя пользователя): gerben Почта для перессылки: rastegind@gmail.com Намерения: хочу научиться собирать пакеты Имя ментора: dutyrok Подписка: dutyrok@altlinux.org
Created attachment 14396 [details] gpg_public_key
Created attachment 14422 [details] gpg_public_key Решил использовать подключ.
Created attachment 14423 [details] gpg_public_key
Created attachment 14424 [details] gpg_public
(Ответ для rastyoginds на комментарий #0) > Создано вложение 14395 [details] [подробности] > ssh_public_key > > Псевдоним (имя пользователя): gerben > Почта для перессылки: rastegind@gmail.com > Намерения: хочу научиться собирать пакеты > Имя ментора: dutyrok > > Подписка: dutyrok@altlinux.org Согласен быть ментором
Ментор есть, ключи в порядке. T/J/S -> 1.3.
Кандидат готов к следующему этапу T/J/M -> 2.0
ssh ключ на gitery.alt зарегистрирован. Адрес для пересылки создан. T/J/S -> 2.3.
На данный момент собран: yyjson - https://git.altlinux.org/people/gerben/packages/?p=libyyjson.git;a=summary
Прошу кандидату дать доступ к сборочнице. T/J/M -> 3.0
ssh ключ на gyle.alt зарегистрирован. Пакет alt-gpgkeys обновлён. Адрес подписан на devel@. T/J/S -> 3.6.
В процессе вступления были собраны: gpg-tui - https://packages.altlinux.org/ru/tasks/349963/ libyyjson - https://packages.altlinux.org/ru/tasks/345401/ python3-module-yyjson - https://packages.altlinux.org/ru/tasks/345401/
Думаю кандидат готов к независимой оценке. Пора звать рецензента.
Призван рецензент (arseny@) для независимой оценки готовности кандидата. T/J/S -> 4.2.
(In reply to Gleb F-Malinovskiy from comment #14) > Призван рецензент (arseny@) для независимой оценки готовности кандидата. > > T/J/S -> 4.2. ACK.
(In reply to rastyoginds from comment #12) > В процессе вступления были собраны: > libyyjson - https://packages.altlinux.org/ru/tasks/345401/ > python3-module-yyjson - https://packages.altlinux.org/ru/tasks/345401/ Здравствуйте, Денис! Пакет libyyjson мне прямо понравился, упакован очень методично и бережно. В частности, докопаться не до чего. :) Впрочем, видимо, сами исходники не очень портили мейнтейнеру нервы (иначе "шрамы боевого опыта" остаются в спеке). Библиотека сразу упакована правильно. К питоньему модулю тоже вопросов не имею, URL и VCS расставлены с умом, pyproject_installer всё сделал. Я хотел бы ещё увидеть, как вы планируете обновлять такие пакеты в сизифе (вышла как минимум новая версия libyyjson). (In reply to rastyoginds from comment #12) > В процессе вступления были собраны: > gpg-tui - https://packages.altlinux.org/ru/tasks/349963/ Это немного позже.
(Ответ для Arseny Maslennikov на комментарий #16) > Я хотел бы ещё увидеть, как вы планируете обновлять такие пакеты в сизифе > (вышла как минимум новая версия libyyjson). Добрый день, Арсений! Спасибо за рецензию, пакеты обновил https://packages.altlinux.org/ru/tasks/362819/
(In reply to Denis Rastyogin from comment #17) > (Ответ для Arseny Maslennikov на комментарий #16) > > Я хотел бы ещё увидеть, как вы планируете обновлять такие пакеты в сизифе > > (вышла как минимум новая версия libyyjson). > > Добрый день, Арсений! Спасибо за рецензию, пакеты обновил > https://packages.altlinux.org/ru/tasks/362819/ Увидел. Неплохо! Питоний модуль даже успел обновиться дважды. :) Вопрос _к кандидату_: если вам вдруг потребуется изменить/улучшить исходники упакованных libyyjson или python3-module-py_yyjson (чего вам пока не приходилось) и упаковать результат, как вы это будете делать?
(Ответ для Arseny Maslennikov на комментарий #18) > Вопрос _к кандидату_: если вам вдруг потребуется изменить/улучшить исходники > упакованных libyyjson или python3-module-py_yyjson (чего вам пока не > приходилось) и упаковать результат, как вы это будете делать? У меня был небольшой опыт при сборке данного пакета https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi-template.git;a=summary . С этими же пакетами я поступил бы также, добавил и применил патч.
(In reply to Denis Rastyogin from comment #19) > (Ответ для Arseny Maslennikov на комментарий #18) > > Вопрос _к кандидату_: если вам вдруг потребуется изменить/улучшить исходники > > упакованных libyyjson или python3-module-py_yyjson (чего вам пока не > > приходилось) и упаковать результат, как вы это будете делать? > > У меня был небольшой опыт при сборке данного пакета > https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi- > template.git;a=summary . Да, так можно. :) Пойду читать gpg-tui на rust+cargo.
(In reply to Denis Rastyogin from comment #19) > (Ответ для Arseny Maslennikov на комментарий #18) > > Вопрос _к кандидату_: если вам вдруг потребуется изменить/улучшить исходники > > упакованных libyyjson или python3-module-py_yyjson (чего вам пока не > > приходилось) и упаковать результат, как вы это будете делать? > > У меня был небольшой опыт при сборке данного пакета > https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi- > template.git;a=summary . С этими же пакетами я поступил бы также, добавил и > применил патч. Ооо. Попалось на глаза: https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi-template.git;a=blob;f=.gear/instead_of_pytests.sh;h=d5cad27c8a986bebf3c1c7d44e72c769f7c15078;hb=982aa4c91056009949ee20c0452aafc62ecbbabb Вот ещё хороший вопрос. Зачем, по-вашему, здесь /bin/bash в hashbang? Есть же /bin/sh.
(Ответ для Arseny Maslennikov на комментарий #21) > Ооо. Попалось на глаза: > https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi- > template.git;a=blob;f=.gear/instead_of_pytests.sh; > h=d5cad27c8a986bebf3c1c7d44e72c769f7c15078; > hb=982aa4c91056009949ee20c0452aafc62ecbbabb > > Вот ещё хороший вопрос. > Зачем, по-вашему, здесь /bin/bash в hashbang? Есть же /bin/sh. Если скрипт строго POSIX-совместим, тогда нужно использовать /bin/sh. Однако так как это пользовательский тест, то можно использовать более функциональный /bin/bash. Конкретно в этом случае не имеет особого значения, что использовать.
(In reply to Denis Rastyogin from comment #22) > Если скрипт строго POSIX-совместим, тогда нужно использовать /bin/sh. Для кода, который предполагает использование только в Альте можно считать, что /bin/sh это такой bash --posix и без libreadline. А зачем вообще тестирование из нескольких строк выносить в отдельный файл? Мне кажется, этот же код нормально смотрелся прямо в секции %check.
(Ответ для Gleb F-Malinovskiy на комментарий #23) > А зачем вообще тестирование из нескольких строк выносить в отдельный файл? > Мне кажется, этот же код нормально смотрелся прямо в секции %check. Мне показалось, что так будет лучше, с точки зрения читабельности spec файла. Да и в какой-то момент тесты могут вырасти с нескольких строчек, поэтому решил сделать небольшой задел на будущее)
(In reply to Denis Rastyogin from comment #22) > (Ответ для Arseny Maslennikov на комментарий #21) > > Ооо. Попалось на глаза: > > https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi- > > template.git;a=blob;f=.gear/instead_of_pytests.sh; > > h=d5cad27c8a986bebf3c1c7d44e72c769f7c15078; > > hb=982aa4c91056009949ee20c0452aafc62ecbbabb > > > > Вот ещё хороший вопрос. > > Зачем, по-вашему, здесь /bin/bash в hashbang? Есть же /bin/sh. > > Если скрипт строго POSIX-совместим, тогда нужно использовать /bin/sh. Однако, > так как это пользовательский тест, А что значит "пользовательский тест"? Тест, наверное, в том смысле, что исполняется в %check с целью как-то протестировать собранное. А что есть "пользовательский"? > то можно использовать более > функциональный /bin/bash. Конкретно в этом случае не имеет особого значения, > что использовать. А раз не имеет значения, то отсюда и из реплики Глеба, как я понимаю, вытекает, что /bin/bash использовать можно, но незачем => не нужно. Более того, даже если в Альте бы поддерживались non-bash реализации sh, они бы все (busybox ash, dash) справились с вашим скриптом. Да и просто многие отличительные башизмы в исходном тексте скриптов труднее использовать правильно, чем функциональность, которая есть и в /bin/sh. Если же позарез потребуются вещи вроде <()-подстановки, к bash можно и вернуться, написав в видном месте, почему. [Написал и после этого проверил; у нас sh, он же sh5, точно умеет <(). Ну, значит, что-нибудь ещё действительно безобидное. А, и <() в хешернице потребует наличия там /proc. :)] В общем, пока что неубедительно; я б предложил далее так не делать, но мчаться срочно исправлять bash на sh тоже смысла нет, такие вещи можно поменять во время следующего обновления. :)
(In reply to Denis Rastyogin from comment #22) > (Ответ для Arseny Maslennikov на комментарий #21) > > Ооо. Попалось на глаза: > > https://git.altlinux.org/people/gerben/packages/?p=python3-module-fastapi- > > template.git;a=blob;f=.gear/instead_of_pytests.sh; > > h=d5cad27c8a986bebf3c1c7d44e72c769f7c15078; > > hb=982aa4c91056009949ee20c0452aafc62ecbbabb > > > > Вот ещё хороший вопрос. > > Зачем, по-вашему, здесь /bin/bash в hashbang? Есть же /bin/sh. > > Если скрипт строго POSIX-совместим, тогда нужно использовать /bin/sh. Однако > так как это пользовательский тест, то можно использовать более > функциональный /bin/bash. Конкретно в этом случае не имеет особого значения, > что использовать. Также, по мотивам: — начать скрипт с `set -e` — хорошая идея; `set -eu` ещё лучше; — `if [ ! -e $item ];`: конкретно тут никаких проблем, но лучше вогнать в привычку брать подстановки в "", чтобы не забыть это сделать там, где подстанавливаются данные внешней природы и возможно появление в команде новых слов после word split. На следующей строке, например, всё хорошо. То же самое справедливо и для .gear/vendor-modules.sh в gpg-tui. Вот любопытный этюд, рекомендую его как-нибудь запустить и посмотреть, что он выведет. :) sh -ex <<'__EOS' D="$(mktemp -d)"; trap "rm -rf $D" 0 1 2 3 6 9 15; cd "$D" bash -ec 'echo [INFO]: Add, update modules' touch N: bash -ec 'echo [INFO]: Add, update modules' touch F: bash -ec 'echo [INFO]: Add, update modules' __EOS BTW, для больших и/или многоразовых программ на шелле есть неплохая идиома: fatal() { printf '%s: %s\n' "$0" "$*" exit 1 } Тогда потом можно писать: [ -e "$item" ] || fatal "$item is missing." Возможны варианты по вкусу, конечно.
(In reply to Denis Rastyogin from comment #12) > В процессе вступления были собраны: > gpg-tui - https://packages.altlinux.org/ru/tasks/349963/ https://git.altlinux.org/tasks/archive/done/_341/349963/gears/300/git?p=git;a=blob;f=.gear/gpg-tui.spec;h=780ae8f9cbda3716bbb77730e3e66c93d14c886f;hb=314e887a72cd73c4c059bd0ed0ca3f5243e19ccc Спек годится. cargo и rustc настроены, актуальные флаги туда переданы, других грубых ошибок imho нет. А вот завендоренные исходники: vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_activeds.a [new file with mode: 0644] blob vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_advapi32.a [new file with mode: 0644] blob vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_advpack.a [new file with mode: 0644] blob vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_amsi.a [new file with mode: 0644] blob vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_api-ms-win-net-isolation-l1-1-0.a [new file with mode: 0644] blob .gear/vendor-modules.sh содержит вызов команды `find`, где из поддерева `vendor/` будто предполагается удалить `*.a` в т. ч., а в коммите e21814d5bcc7f82e4d6b360d96cce326d9bc75f9 вот такое выше. Это как? Nits: Ман-страницы указаны в расчёте на то, что кто-нибудь их перед упаковкой в rpm сожмёт (smth.1.*). Интересно, почему так? (это не экзамен, на этот вопрос нет разумных неправильных ответов, хочу логику мышления увидеть)
(In reply to Arseny Maslennikov from comment #26) > BTW, для больших и/или многоразовых программ на шелле есть неплохая идиома: > fatal() { > printf '%s: %s\n' "$0" "$*" > exit 1 > } printf >&2, конечно, если уж зашла речь об идиомах.
(Ответ для Arseny Maslennikov на комментарий #25) Спасибо за подробную резензию, из неё я продчеркнул несколько моментов, о которых раньше не задумывался) > А что значит "пользовательский тест"? > Тест, наверное, в том смысле, что исполняется в %check с целью как-то > протестировать собранное. А что есть "пользовательский"? Под пользовательским имелось ввиду то что это вроде проверки "для себя", чтобы убедиться, что собранный пакет работает в сборочной среде. То есть это не часть общепринятого стандарта тестирования, а что-то кастомное, написанное под конкретный случай, из-за невозможности применить тесты из upstream. Я учту предложенные изменения при следующем обновлении пакета, спасибо!
(Ответ для Arseny Maslennikov на комментарий #27) > .gear/vendor-modules.sh содержит вызов команды `find`, где из поддерева > `vendor/` будто предполагается удалить `*.a` в т. ч., а в коммите > e21814d5bcc7f82e4d6b360d96cce326d9bc75f9 вот такое выше. Это как? С завендоренными исходниками действительно произошел небольшой косяк. Сначала появились vendors, а затем добавился скрипт для автоматизации. Только на этапе работы со скриптом я обратил внимание на файлы вроде vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_activeds.a [новый файл с правами 0644] итд. В общем, скрипт не успел отработать до создания коммита) > Nits: > Ман-страницы указаны в расчёте на то, что кто-нибудь их перед упаковкой в > rpm сожмёт (smth.1.*). Интересно, почему так? (это не экзамен, на этот > вопрос нет разумных неправильных ответов, хочу логику мышления увидеть) В пакете rpm-build есть scripts/brp-compress.in:88: @RPMCONFIGDIR@/compress_files "$f", (https://git.altlinux.org/gears/r/rpm-build.git?p=rpm-build.git;a=blob;f=scripts/brp-compress.in;h=3e165adddfb7e5e0e65cbfe51094675049391ee2;hb=6496f05484ea73b4e97f32b376b4f03f56c3c5cf#:~:text=88-,%40RPMCONFIGDIR%40,-/compress_files%C2%A0%22%24f%22) Думаю, именно из-за этого происходит автоматическое сжатие man-страниц перед упаковкой в RPM.
(In reply to Dmitry V. Levin from comment #28) > (In reply to Arseny Maslennikov from comment #26) > > BTW, для больших и/или многоразовых программ на шелле есть неплохая идиома: > > fatal() { > > printf '%s: %s\n' "$0" "$*" > > exit 1 > > } > > printf >&2, конечно, если уж зашла речь об идиомах. Да, вне всяких сомнений.
(In reply to Denis Rastyogin from comment #29) > (Ответ для Arseny Maslennikov на комментарий #25) > Спасибо за подробную резензию, из неё я продчеркнул несколько моментов, о > которых раньше не задумывался) :) > > А что значит "пользовательский тест"? > > Тест, наверное, в том смысле, что исполняется в %check с целью как-то > > протестировать собранное. А что есть "пользовательский"? > > Под пользовательским имелось ввиду то что это вроде проверки "для себя", > чтобы убедиться, что собранный пакет работает в сборочной среде. > То есть это не часть общепринятого стандарта тестирования, а что-то > кастомное, написанное под конкретный случай, из-за невозможности применить > тесты из upstream. Ага, понятно, благодарю за пояснение. "Пользовательский" в смысле non-first-party. Слово "пользовательский" здесь как-то не подходит. Но ладно, главное, мы поняли, о чём речь. :) Никакого "общепринятого стандарта тестирования" нет и скорее не может быть: ПО разное, модели определения мест в коде, где могут проявиться сбои или быть допущены ошибки, разные. Вот стандартизованный API чего-то можно себе представить, или стандартный протокол общения между тестом и test harness. Тесты от апстрима ценны не сами по себе, а тем, что (как можно было бы полагать) апстрим хорошо знает свой код и знает, что тестировать — т. е. некомпетентный апстрим может в качестве тестов всякие глупости писать. Или же просто разработчики из какой-то другой реальности могут написать что-то несовместимое с окружением на альт-сборочнице, хоть среди СПО в целом и в наше время в частности такое встречается всё реже. Из того же, что скрипт написан под конкретные обстоятельства, не следует, что он "для себя" — результаты его работы всё же для пользователей — и что к нему должна быть заниженная планка требований по качеству, мол, "если мой плохой код вижу только я, да ещё и редко запускаю, то и ладно". Можно, конечно, на более высоком уровне абстракции опытным взглядом решить, что и так сойдёт в конкретных текущих условиях задачи, и не тратить времени на обретение более элегантного или корректного решения, написать покривее; здесь это не просто кривой код, а сознательное решение, которое позволило выиграть что-то другое. Но sh -> bash на пустом месте — не тот случай. Выражение о "плохом коде", что я позволил себе выше, относится не к конкретному вашему пакету и коду (в частности, у вас этого всё же было не много), а абстрактно и вообще. > Я учту предложенные изменения при следующем обновлении пакета, спасибо! Добро. :)
(In reply to Denis Rastyogin from comment #30) > (Ответ для Arseny Maslennikov на комментарий #27) > > .gear/vendor-modules.sh содержит вызов команды `find`, где из поддерева > > `vendor/` будто предполагается удалить `*.a` в т. ч., а в коммите > > e21814d5bcc7f82e4d6b360d96cce326d9bc75f9 вот такое выше. Это как? > > С завендоренными исходниками действительно произошел небольшой косяк. > Сначала появились vendors, а затем добавился скрипт для автоматизации. > > Только на этапе работы со скриптом я обратил внимание на файлы вроде > vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_activeds.a [новый файл с > правами 0644] итд. > В общем, скрипт не успел отработать до создания коммита) Такое бывает. Прошу выпустить (тоже в сизиф, раз задание уже закоммитилось) новый релиз пакета, при подготовке которого уже воспользоваться своим инструментом и добиться результата. :)
(In reply to Denis Rastyogin from comment #30) > (Ответ для Arseny Maslennikov на комментарий #27) > > Nits: > > Ман-страницы указаны в расчёте на то, что кто-нибудь их перед упаковкой в > > rpm сожмёт (smth.1.*). Интересно, почему так? (это не экзамен, на этот > > вопрос нет разумных неправильных ответов, хочу логику мышления увидеть) > > В пакете rpm-build есть scripts/brp-compress.in:88: > @RPMCONFIGDIR@/compress_files "$f", > (https://git.altlinux.org/gears/r/rpm-build.git?p=rpm-build.git;a=blob; > f=scripts/brp-compress.in;h=3e165adddfb7e5e0e65cbfe51094675049391ee2; > hb=6496f05484ea73b4e97f32b376b4f03f56c3c5cf#:~:text=88-,%40RPMCONFIGDIR%40,-/ > compress_files%C2%A0%22%24f%22) > Думаю, именно из-за этого происходит автоматическое сжатие man-страниц перед > упаковкой в RPM. Я выше отмечал, что на заданный вопрос нет разумных неправильных ответов. Хоть ваш ответ и верен фактически (что хорошо), он не годится, потому что он не на тот вопрос :/ Имел в виду вот что: интересно, почему man-страницы указаны в %files в расчёте на то, что их потом сожмут. Я считаю, что лучше написать `smth.1*` (но не считаю себя вправе настаивать). Согласны ли вы? Если нет, то почему?
(In reply to Arseny Maslennikov from comment #34) > интересно, почему man-страницы указаны в %files в > расчёте на то, что их потом сожмут. Т. е. указаны таким шаблоном, который сломается, если brp-compress нет.
(Ответ для Arseny Maslennikov на комментарий #33) > Такое бывает. > Прошу выпустить (тоже в сизиф, раз задание уже закоммитилось) новый релиз > пакета, при подготовке которого уже воспользоваться своим инструментом и > добиться результата. :) Подготовил задание с исправлением https://packages.altlinux.org/ru/tasks/363382/
(Ответ для Arseny Maslennikov на комментарий #34) > Имел в виду вот что: интересно, почему man-страницы указаны в %files в > расчёте на то, что их потом сожмут. > Я считаю, что лучше написать `smth.1*` (но не считаю себя вправе > настаивать). Согласны ли вы? Если нет, то почему? Согласен, тк это более универсальный вариант. В случае отказа от сжатия man страниц, пакет не сломается, если будет `smth.1*`. Однако, помимо моего пакета, существует множество других пакетов, которые рассчитывают на сжатие man-страниц. Поэтому, на мой взгляд, если такой переход и произойдёт, то он окажется небесшовным.
(In reply to Denis Rastyogin from comment #36) > (Ответ для Arseny Maslennikov на комментарий #33) > > Такое бывает. > > Прошу выпустить (тоже в сизиф, раз задание уже закоммитилось) новый релиз > > пакета, при подготовке которого уже воспользоваться своим инструментом и > > добиться результата. :) > > Подготовил задание с исправлением > https://packages.altlinux.org/ru/tasks/363382/ Все придирки к программам на шелле в составе fastapi-template относятся к любому пакету, в том числе и сюда. Поправьте грубые ошибки в `.gear/vendor-modules.sh`, пожалуйста. Грубейшую из них (нет `>&2`) обнаружил вообще не я, а ув. ldv@. Shortlog будет выглядеть как-то наподобие: > vendor-modules.sh: fix quoting and error logging > Remove unnecessary vendored files > 0.11.0-alt2 (в порядке от предков к потомкам; git log, gitweb и визуализаторы графа показывают наоборот) Кроме того, нам с вами наконец-то довелось составить нетривиальный changelog: > %changelog > +* Mon Nov 25 2024 Denis Rastyogin <gerben@altlinux.org> 0.11.0-alt2 > +- remove unnecessary vendor files. > +- optimize file pattern for man pages. > + 1) Стили оформления changelog встречаются разные, здесь, в новом пакете, вы задаёте стиль сами, но о совершённых действиях принято писать в прошедшем времени. Т. е. optimized, removed. 2) changelog пакета — это место для замечаний не о сопровождении пакета (как получается в gear git shortlog), а о его наполнении для пользователей. У вас 2 записи; первая, наверное, касается пользователей (хотя это под вопросом, но хоть что-нибудь в changelog должно быть), а вторая точно нет. О том, что вы optimized file pattern for man pages, разработчики с удовольствием прочтут в git-коммитах. :) Прошу тоже поправить.
(Ответ для Arseny Maslennikov на комментарий #38) > (In reply to Denis Rastyogin from comment #36) > > (Ответ для Arseny Maslennikov на комментарий #33) > > > Такое бывает. > > > Прошу выпустить (тоже в сизиф, раз задание уже закоммитилось) новый релиз > > > пакета, при подготовке которого уже воспользоваться своим инструментом и > > > добиться результата. :) > > > > Подготовил задание с исправлением > > https://packages.altlinux.org/ru/tasks/363382/ > Все придирки к программам на шелле в составе fastapi-template относятся к > любому пакету, в том числе и сюда. Поправьте грубые ошибки в > `.gear/vendor-modules.sh`, пожалуйста. > Грубейшую из них (нет `>&2`) обнаружил вообще не я, а ув. ldv@. > > Shortlog будет выглядеть как-то наподобие: > > vendor-modules.sh: fix quoting and error logging > > Remove unnecessary vendored files > > 0.11.0-alt2 > (в порядке от предков к потомкам; git log, gitweb и визуализаторы графа > показывают наоборот) > > Кроме того, нам с вами наконец-то довелось составить нетривиальный changelog: > > %changelog > > +* Mon Nov 25 2024 Denis Rastyogin <gerben@altlinux.org> 0.11.0-alt2 > > +- remove unnecessary vendor files. > > +- optimize file pattern for man pages. > > + > 1) Стили оформления changelog встречаются разные, здесь, в новом пакете, вы > задаёте стиль сами, но о совершённых действиях принято писать в прошедшем > времени. Т. е. optimized, removed. > 2) changelog пакета — это место для замечаний не о сопровождении пакета (как > получается в gear git shortlog), а о его наполнении для пользователей. У вас > 2 записи; первая, наверное, касается пользователей (хотя это под вопросом, > но хоть что-нибудь в changelog должно быть), а вторая точно нет. О том, что > вы optimized file pattern for man pages, разработчики с удовольствием > прочтут в git-коммитах. :) > Прошу тоже поправить. Спасибо за разъяснения. Поправил моменты, которые ранее упустил. https://packages.altlinux.org/ru/tasks/363382/
(In reply to Denis Rastyogin from comment #39) > (Ответ для Arseny Maslennikov на комментарий #38) > > Прошу тоже поправить. > > Спасибо за разъяснения. Поправил моменты, которые ранее упустил. > https://packages.altlinux.org/ru/tasks/363382/ Хорошо. (In reply to Denis Rastyogin from comment #37) > (Ответ для Arseny Maslennikov на комментарий #34) > > Имел в виду вот что: интересно, почему man-страницы указаны в %files в > > расчёте на то, что их потом сожмут. > > Я считаю, что лучше написать `smth.1*` (но не считаю себя вправе > > настаивать). Согласны ли вы? Если нет, то почему? > > Согласен, тк это более универсальный вариант. В случае отказа от сжатия man > страниц, пакет не сломается, если будет `smth.1*`. Однако, помимо моего > пакета, существует множество других пакетов, которые рассчитывают на сжатие > man-страниц. Поэтому, на мой взгляд, если такой переход и произойдёт, то он > окажется небесшовным. Валидное рассуждение, спасибо. :)
К представленным на review пакетам вопросов больше нет; предлагаю для закрепления результата обновить что-нибудь заброшенное и при этом не наделать грубых ошибок. Вот, например, пакет, который не мейнтейнится, а достаётся инструментом fcimport (который делает нечто стыдное, не буду говорить, что, хотя и так понятно): https://packages.altlinux.org/sisyphus/srpms/librtaudio Его стоило бы обновить и собрать правильно (иными словами, определить, что там сейчас неправильного, и поправить). Вот его апстрим: https://github.com/thestk/rtaudio Мне этот пакет видится весьма хорошим экзерсисом. :) Но вы с ув. ментором можете выбрать что-то ещё, конечно. P.S. У rtaudio есть собрат rtmidi, который упакован в альт и честно сопровождается.
(Ответ для Arseny Maslennikov на комментарий #41) > Вот, например, пакет, который не мейнтейнится, а достаётся инструментом > fcimport (который делает нечто стыдное, не буду говорить, что, хотя и так > понятно): > https://packages.altlinux.org/sisyphus/srpms/librtaudio > Его стоило бы обновить и собрать правильно (иными словами, определить, что > там сейчас неправильного, и поправить). > Вот его апстрим: https://github.com/thestk/rtaudio > Мне этот пакет видится весьма хорошим экзерсисом. :) Но вы с ув. ментором > можете выбрать что-то ещё, конечно. > > P.S. У rtaudio есть собрат rtmidi, который упакован в альт и честно > сопровождается. Пересобрал и обновил librtaudio в задании https://packages.altlinux.org/ru/tasks/364130/ P.S. Пришлось немного повозиться, так как пакет OPN2BankEditor, использующий эту библиотеку, оказался немного несовместим с её новой версией.
(In reply to Denis Rastyogin from comment #42) > (Ответ для Arseny Maslennikov на комментарий #41) > > Вот, например, пакет, который не мейнтейнится, а достаётся инструментом > > fcimport (который делает нечто стыдное, не буду говорить, что, хотя и так > > понятно): > > https://packages.altlinux.org/sisyphus/srpms/librtaudio > > Его стоило бы обновить и собрать правильно (иными словами, определить, что > > там сейчас неправильного, и поправить). > > Вот его апстрим: https://github.com/thestk/rtaudio > > Мне этот пакет видится весьма хорошим экзерсисом. :) Но вы с ув. ментором > > можете выбрать что-то ещё, конечно. > > > > P.S. У rtaudio есть собрат rtmidi, который упакован в альт и честно > > сопровождается. > > Пересобрал и обновил librtaudio в задании > https://packages.altlinux.org/ru/tasks/364130/ Вижу много логичных исправлений по сравнению с 5.2.0 и меньше. У пакета с библиотекой изменилось имя, что хорошо. Патчи и коммиты оформлены грамотно. > P.S. Пришлось немного повозиться, так как пакет OPN2BankEditor, использующий > эту библиотеку, оказался немного несовместим с её новой версией. Вы попытались и его починить, спасибо! ^^ Ситуация была такова, что часть пакетов ожидает API от пятой версии, а часть от шестой (нередкая ситуация в большом репозитории). Перед вами стоял выбор: — собрать обе версии в разных исходных пакетах; — оставить только более свежую и сделать что-нибудь с пакетами, которым это не нравится. Пошли по второму пути; что OK как решение и может вас хорошо характеризовать как разработчика. Но вот характер исправления противоестественный: вы патчите и пакета-пользователя, и библиотеку. Если пополнять код библиотеки особыми знаниями про ужимки и особенности каждого проекта, который от неё зависит, то он раздуется до немыслимых размеров. Библиотеки в силу своей сути не должны превращаться в коллекцию хаков-подстроек под пользующийся ими код, потому что "вас у меня много, а я одна", их задача — дать программисту переиспользуемый механизм решения подзадачи. Иначе — это превращение в Windows (её даже в Microsoft не могут полностью пересобрать из исходников, dllки, как священные коровы, кочуют из релиза в релиз). Представьте себе, если Qt/Glib начнут себе наворачивать мегабайты хаков для каждого пакета, что от них зависит. Тем более, авторы библиотеки между 5 и 6 (soname 6 и 7) явно избавили свой интерфейс от с++-исключений, о чём написали в свои doxygen-файлы; у них, как обычно, были на это причины. Ну и зачем обратно добавлять? Т. е., идя вашим путём, стоило запатчить только opn2bankeditor: заменить RtAudioError на то, что вместо него в новой librtaudio, и наложить другой ваш патч. :) Другой вариант — пересобрать opn2bankeditor так, чтобы он пользовался bundled rtaudio: взять зафиксированный сабмодуль у авторов и подложить его как новый source. Такое у нас в целом не приветствуется, но входит в моду у апстримов прикладных пакетов (один cargo-культ чего стоит, он же "синдром leftpad"). Да и библиотека из двух файлов большой погоды не сделает, её и прочесть можно. Короче, я против того, чтобы возвращать исключения в rtaudio 6. Ещё мелочёвка: * в changelog-записи: что значит "rebuilt as legacy library"?
(Ответ для Arseny Maslennikov на комментарий #43) > Вижу много логичных исправлений по сравнению с 5.2.0 и меньше. У пакета с > библиотекой изменилось имя, что хорошо. Патчи и коммиты оформлены грамотно. > > > P.S. Пришлось немного повозиться, так как пакет OPN2BankEditor, использующий > > эту библиотеку, оказался немного несовместим с её новой версией. > Вы попытались и его починить, спасибо! ^^ > > Ситуация была такова, что часть пакетов ожидает API от пятой версии, а часть > от шестой (нередкая ситуация в большом репозитории). > Перед вами стоял выбор: > — собрать обе версии в разных исходных пакетах; > — оставить только более свежую и сделать что-нибудь с пакетами, которым это > не нравится. > Пошли по второму пути; что OK как решение и может вас хорошо характеризовать > как разработчика. > > Но вот характер исправления противоестественный: вы патчите и > пакета-пользователя, и библиотеку. > > Если пополнять код библиотеки особыми знаниями про ужимки и особенности > каждого проекта, который от неё зависит, то он раздуется до немыслимых > размеров. > Библиотеки в силу своей сути не должны превращаться в коллекцию > хаков-подстроек под пользующийся ими код, потому что "вас у меня много, а я > одна", их задача — дать программисту переиспользуемый механизм решения > подзадачи. > Иначе — это превращение в Windows (её даже в Microsoft не могут полностью > пересобрать из исходников, dllки, как священные коровы, кочуют из релиза в > релиз). Представьте себе, если Qt/Glib начнут себе наворачивать мегабайты > хаков для каждого пакета, что от них зависит. > > Тем более, авторы библиотеки между 5 и 6 (soname 6 и 7) явно избавили свой > интерфейс от с++-исключений, о чём написали в свои doxygen-файлы; у них, как > обычно, были на это причины. Ну и зачем обратно добавлять? > > Т. е., идя вашим путём, стоило запатчить только opn2bankeditor: заменить > RtAudioError на то, что вместо него в новой librtaudio, и наложить другой > ваш патч. :) > Другой вариант — пересобрать opn2bankeditor так, чтобы он пользовался > bundled rtaudio: взять зафиксированный сабмодуль у авторов и подложить его > как новый source. Такое у нас в целом не приветствуется, но входит в моду у > апстримов прикладных пакетов (один cargo-культ чего стоит, он же "синдром > leftpad"). Да и библиотека из двух файлов большой погоды не сделает, её и > прочесть можно. > > Короче, я против того, чтобы возвращать исключения в rtaudio 6. Исправил в задании https://packages.altlinux.org/ru/tasks/364130/. Я согласен с выше сказанным, но хочу немного оправдаться. Несостыковка произошла из-за того, что клиент не успели адаптировать под новую версию библиотеки, и я надеюсь, что это временная проблема. В будущем upstream OPN2BankEditor, который, как я понимаю, ещё не совсем заброшен, исправит её самостоятельно. Поэтому свои патчи я рассматривал с учётом этого и выбрал путь наименьшего сопротивления, добавив старый класс. Конечно, апстрим может и не исправить проблему, и пакетов с подобными клиентами может быть много, а изменения могли бы составить и больше 20 строк, но, рассматривая конкретно этот случай, я посчитал своё решение приемлемым в данных условиях. > Ещё мелочёвка: > * в changelog-записи: что значит "rebuilt as legacy library"? Подсмотрел у одного из мейнтейнеров запись при подобных изменениях и посчитал её корректной для моего случая)
Извините за тормоз с моей стороны: мне потребовалось найти довольно много времени, чтобы вчитаться в этот неладный opn2bankeditor и понять, как его исправлять. Когда я предлагал обновить librtaudio, я не думал, что придётся _столько_ разбираться в побочно возникающих вопросах (для кандидата на join, который ещё и ушёл на review, а не с ментором занимается, это перебор). Ну ничего, бо́льшую часть пути мы уже прошли. :)
https://git.altlinux.org/tasks/364130/gears/700/git?p=git;a=blob;f=.gear/patches/fix-openStream-callback-arguments.patch;h=85148719bfe424071925a8ad1f15edbf7a71469d;hb=3463721aba7e044c116adf549c850db2b557dab9 Здесь мы убираем errorCallback совсем. Больше он по коду нигде не используется. Может, эту функцию убрать? (Жалко, Wunused-function ничего не показал, это же не private-метод). Более того, AudioOutRt::errorCallback(RtAudioError::Type, const std::string&) — единственное место во всём проекте, где кидают исключение. После исчезновения этой функции throw statement в коде отсутствует (будем считать, что в Qt их нет, да и в любом случае в src/audio/ao_rtaudio.cpp обращений к Qt нет). Может, там и ловить ничего не надо, только проверять всплывающий код ошибки? Предлагаю уйти от try/catch в AudioOutRt::AudioOutRt(*) вообще. Больше у меня вопросов к пакету не будет. :) Все наши мытарства вокруг кода OPN2BankEditor никак на вердикт моего review не влияют, они не относятся к упаковке и ведению сизифа напрямую.
(Ответ для Arseny Maslennikov на комментарий #46) > https://git.altlinux.org/tasks/364130/gears/700/git?p=git;a=blob;f=.gear/ > patches/fix-openStream-callback-arguments.patch; > h=85148719bfe424071925a8ad1f15edbf7a71469d; > hb=3463721aba7e044c116adf549c850db2b557dab9 > Здесь мы убираем errorCallback совсем. Больше он по коду нигде не > используется. Может, эту функцию убрать? (Жалко, Wunused-function ничего не > показал, это же не private-метод). > Более того, AudioOutRt::errorCallback(RtAudioError::Type, const > std::string&) — единственное место во всём проекте, где кидают исключение. > После исчезновения этой функции throw statement в коде отсутствует (будем > считать, что в Qt их нет, да и в любом случае в src/audio/ao_rtaudio.cpp > обращений к Qt нет). Может, там и ловить ничего не надо, только проверять > всплывающий код ошибки? Предлагаю уйти от try/catch в > AudioOutRt::AudioOutRt(*) вообще. > > Больше у меня вопросов к пакету не будет. :) > Все наши мытарства вокруг кода OPN2BankEditor никак на вердикт моего review > не влияют, они не относятся к упаковке и ведению сизифа напрямую. Исправил в задании https://packages.altlinux.org/ru/tasks/364130/
(In reply to Denis Rastyogin from comment #47) > (Ответ для Arseny Maslennikov на комментарий #46) > > https://git.altlinux.org/tasks/364130/gears/700/git?p=git;a=blob;f=.gear/ > > patches/fix-openStream-callback-arguments.patch; > > h=85148719bfe424071925a8ad1f15edbf7a71469d; > > hb=3463721aba7e044c116adf549c850db2b557dab9 > > Здесь мы убираем errorCallback совсем. Больше он по коду нигде не > > используется. Может, эту функцию убрать? (Жалко, Wunused-function ничего не > > показал, это же не private-метод). > > Более того, AudioOutRt::errorCallback(RtAudioError::Type, const > > std::string&) — единственное место во всём проекте, где кидают исключение. > > После исчезновения этой функции throw statement в коде отсутствует (будем > > считать, что в Qt их нет, да и в любом случае в src/audio/ao_rtaudio.cpp > > обращений к Qt нет). Может, там и ловить ничего не надо, только проверять > > всплывающий код ошибки? Предлагаю уйти от try/catch в > > AudioOutRt::AudioOutRt(*) вообще. > > > > Больше у меня вопросов к пакету не будет. :) > > Все наши мытарства вокруг кода OPN2BankEditor никак на вердикт моего review > > не влияют, они не относятся к упаковке и ведению сизифа напрямую. > > Исправил в задании https://packages.altlinux.org/ru/tasks/364130/ Спасибо! > diff --git a/OPN2BankEditor.spec b/OPN2BankEditor.spec > index b74bde5..4d3ec3c 100644 > --- a/OPN2BankEditor.spec > +++ b/OPN2BankEditor.spec > @@ -3,13 +3,13 @@ > > Name: OPN2BankEditor > Version: 1.3.0.90.git64f4a24 > -Release: alt1 > +Release: alt2 > > Summary: a small cross-platform editor for OPN2 FM banks > > License: GPL-3.0 > Group: Sound > -URL: https://github.com/Wohlstand/OPN2BankEditor > +Url: https://github.com/Wohlstand/OPN2BankEditor Не возражаю; мне скорее без особой разницы. rpm-build принимает обе формы, так что ладно. > > BuildRequires(pre): rpm-macros-cmake > BuildRequires: cmake В остальном вопросов нет. :)
Ну что. Денис научился сопровождать разнородные пакеты, в том числе нетривиально их меняя; показал, что понимает, что делает, и что способен в позиции мейнтейнера принимать обоснованные решения; хорошо себя проявил в общении в команде. Думаю, он может стать членом Team. ____ Более тонкие нюансы, например, ведения changelog в пакете приходят с опытом, если притрагиваться к принципиально разным пакетам, а тем более — если иногда выступать и разработчиком оригинального проекта.
Пользователь добавлен в группу мейнтейнеров. Желаю удачного мейнтейнерства!
