https://bugzilla.redhat.com/show_bug.cgi?id=2310622 --- Comment #23 from Peter Lemenkov <lemenkov@xxxxxxxxx> --- Here are things which require your attention. * Consider removing outdated "Group: Devel" field. It's not a blocker but we stopped using it since Fedora 25. if you consider keeping it then the value is wrong. Here is a list of a valid values - https://fedoraproject.org/wiki/RPMGroups. * %license field needs some love. We need to reflect licensing information for *all* source files used to build artifacts we ship in a package. I see that most of the source code is licensed under GPLv3+ but at least part of the application is derived from flashrom source code which is licensed under GPLv2+ and some source files licensed under LGPLv2.1. Here is a licensing breakdown: ``` GNU General Public License v2.0 or later ---------------------------------------- ./IMSProg_editor/IMSProg_editor.pro ./IMSProg_editor/delegates.cpp ./IMSProg_editor/delegates.h ./IMSProg_editor/ezp_chip_editor.cpp ./IMSProg_editor/ezp_chip_editor.h ./IMSProg_programmer/ch341a_gpio.c ./IMSProg_programmer/ch341a_gpio.h ./IMSProg_programmer/ch341a_spi.c ./IMSProg_programmer/ch341a_spi.h ./IMSProg_programmer/dialogrp.h ./IMSProg_programmer/dialogsp.cpp ./IMSProg_programmer/dialogsp.h ./IMSProg_programmer/flashcmd_api.c ./IMSProg_programmer/flashcmd_api.h ./IMSProg_programmer/i2c_eeprom.c ./IMSProg_programmer/i2c_eeprom_api.h ./IMSProg_programmer/main.cpp ./IMSProg_programmer/mw_eeprom.c ./IMSProg_programmer/mw_eeprom_api.h ./IMSProg_programmer/nandcmd_api.h ./IMSProg_programmer/snorcmd_api.h ./IMSProg_programmer/spi_eeprom.c ./IMSProg_programmer/spi_eeprom.h ./IMSProg_programmer/spi_eeprom_api.h ./IMSProg_programmer/spi_nor_flash.c ./IMSProg_programmer/timer.c ./IMSProg_programmer/timer.h ./IMSProg_programmer/types.h GNU General Public License v2.0 or later [obsolete FSF postal address (Mass Ave)] --------------------------------------------------------------------------------- ./IMSProg_programmer/bitbang_microwire.c ./IMSProg_programmer/bitbang_microwire.h GNU General Public License v3.0 or later ---------------------------------------- ./IMSProg_programmer/ch341a_i2c.c ./IMSProg_programmer/ch341funcs.c ./IMSProg_programmer/dialogabout.cpp ./IMSProg_programmer/dialogabout.h ./IMSProg_programmer/dialoginfo.cpp ./IMSProg_programmer/dialoginfo.h ./IMSProg_programmer/dialogrp.cpp ./IMSProg_programmer/dialogsetaddr.cpp ./IMSProg_programmer/dialogsetaddr.h ./IMSProg_programmer/dialogsfdp.cpp ./IMSProg_programmer/dialogsfdp.h ./IMSProg_programmer/dialogsr.cpp ./IMSProg_programmer/dialogsr.h ./IMSProg_programmer/mainwindow.cpp ./IMSProg_programmer/mainwindow.h GNU Lesser General Public License, Version 2.1 ---------------------------------------------- ./IMSProg_programmer/chunks.cpp ./IMSProg_programmer/commands.cpp ./IMSProg_programmer/qhexedit.cpp ./IMSProg_programmer/searchdialog.cpp ./IMSProg_programmer/searchdialog.h ``` So a proper %license field value has to be "GPL-2.0-or-later AND GPL-3.0-or-later AND LGPL-2.1-only". * A small proposal - please move these lines from %prep section to %build section: ``` # update translations lrelease-qt5 IMSProg_editor/language/*.ts lrelease-qt5 IMSProg_programmer/language/*.ts ``` Sometimes it's very handy to run "rpmbuild -bp ./your.spec" without installing any dependencies. Just to check how patches are applied, look through unpacked source code, etc. basically everything which doesn't create a new build artifacts (sources patching, preparatory file renaming, etc - not very invasive) should go to %prep sections. Artifacts should be generated in %build section. * Your package installs desktop-files. We require that they have to be validated with desktop-file-validate. The same story with metainfo files - you have to validate them with appstream-util. Validation can be done in %install or %check sections at your choice. Take a look at these spec-files as a possible examples. ** https://src.fedoraproject.org/rpms/bitcoin-core/blob/rawhide/f/bitcoin-core.spec#_263 ** https://src.fedoraproject.org/rpms/mpv/blob/rawhide/f/mpv.spec#_189 ** https://src.fedoraproject.org/rpms/xchm/blob/rawhide/f/xchm.spec#_35 * A small doc-related issue. It seems that you listed %_docdir/imsprog/ explicitly. I don't think it is necessary. The contents of this directory is included already. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2310622 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202310622%23c23 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
