Spec URL: https://heliocastro.fedorapeople.org/lxqt/lxqt-panel.spec SRPM URL: https://heliocastro.fedorapeople.org/lxqt/lxqt-panel-0.8.0-2.fc21.src.rpm Description: Main panel bar for LXQt desktop suite Fedora Account System Username: heliocastro
Very quick look at your spec file: * Please check if the license tag should "LGPLv2" or "LGPLv2+" see: https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F * Basically dependency for packages made from the same srpm should be isa specific https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Requires_2 * Please explain where lots of "Requires (not BuildRequires): pkgconfig(foo)" in -devel package comes from - Note that if the package contains pkgconfig .pc files, dependency for such pkgconfig files are automatically generated on rpmbuild process so usually no need to explicitly write them. Anyway pkgconfig(foo) is meant for pkgconfig file related dependency and currently I don't see such Requries is needed for lxqt-panel-devel package * Please beaware of directory ownership issue https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_and_Directory_Ownership - Please check the ownership of - {_libdir}/lxqt-panel/ - What owns %{_sysconfdir}/xdg/lxqt/ ? - It seems that multiple package owns - %{_datadir}/lxqt-qt5 - %{_includedir}/lxqt Please examine which package should own these directories and fix files lists properly * Files under /etc is configuration files and should be marked as %config(noreplace) http://www.pathname.com/fhs/pub/fhs-2.3.html#ETCHOSTSPECIFICSYSTEMCONFIGURATION https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Configuration_files * Why does this package obsoletes gtk version lxpanel? It should be parallel installable and this should not obsolete lxpanel.
By the way, please specify review request dependency (i.e. to make this package build, which package should be reviewed in advance?) using "Depends On:"
All comments issues fixed in https://heliocastro.fedorapeople.org/lxqt/lxqt-panel-0.8.0-3.fc21.src.rpm Policy allows multiple packages owns same directory Depends On: liblxqt
naming: ok license: ok, but could be ammended to License: LGPLv2+ sources: ok 2d2c2251659f285031f65bfb30c741c3 lxqt-panel-0.8.0.tar.xz macros: ok scriptlets: ok (n/a) 1. SHOULD omit Obsoletes: lxpanel until an upgrade path/plan is agreed upon 2. SHOULD omit -devel dependencies: Requires: pkgconfig(Qt5Widgets) Requires: pkgconfig(Qt5DBus) Requires: pkgconfig(Qt5Help) Requires: pkgconfig(Qt5Xml) Requires: pkgconfig(Qt5X11Extras) Requires: pkgconfig(lxqt-qt5) Requires: pkgconfig(lxqtmount-qt5) Requires: pkgconfig(lxqt-globalkeys-qt5) Requires: pkgconfig(lxqt-globalkeys-ui-qt5) these ought to get detected automatically from included .pc files already 3. MUST own %{_libdir}/lxqt-panel/ dir (unless something else owns it?0 4. %files devel %{_includedir}/lxqt headers only, no libs? If so, could consider making this subpkg noarch or simply not ship these at all for now. Please fix at least the MUST blocker items, and give fresh spec/srpm links
Fixes in: https://heliocastro.fedorapeople.org/lxqt/lxqt-panel-0.8.0-4.fc21.src.rpm https://heliocastro.fedorapeople.org/lxqt/lxqt-panel.spec
3 is apparently still not fixed, %files only has: %{_libdir}/lxqt-panel/*.so
Oops. Fixed on: https://heliocastro.fedorapeople.org/lxqt/lxqt-panel-0.8.0-5.fc21.src.rpm https://heliocastro.fedorapeople.org/lxqt/lxqt-panel.spec
Good, thanks, APPROVED
New Package SCM Request ======================= Package Name: lxqt-panel Short Description: Main panel bar for LXQt desktop suite Upstream URL: http://lxqt.org Owners: heliocastro rdieter tieugene Branches: f20 f21 el6 epel7 InitialCC: heliocastro
Git done (by process-git-requests).
imported