Spec URL: https://tieugene.fedorapeople.org/rpms/liblxqt/liblxqt.spec SRPM URL: https://tieugene.fedorapeople.org/rpms/liblxqt/liblxqt-0.8.0-1.fc20.src.rpm Description: Core utility library for all LXQT components Fedora Account System Username: tieugene Koji builds: http://koji.fedoraproject.org/koji/taskinfo?taskID=7945877 (F22) http://koji.fedoraproject.org/koji/taskinfo?taskID=7945892 (F21)
Updated specs with merge from Eugene original package Spec URL: SRPM URL: https://heliocastro.fedorapeople.org/lxqt/liblxqt-0.8.0-3.fc21.src.rpm Description: https://heliocastro.fedorapeople.org/lxqt/liblxqt.spec Fedora Account System Username: heliocastro
Missing description Spec: https://heliocastro.fedorapeople.org/lxqt/liblxqt.spec SRPM URL: https://heliocastro.fedorapeople.org/lxqt/liblxqt-0.8.0-3.fc21.src.rpm Description: Core Shared library for LXQt desktop suite Fedora Account System Username: heliocastro
*** Bug 1151711 has been marked as a duplicate of this bug. ***
helio, all those Requires: pkgconfig in -devel pkg should get autodetected (so you shouldn't need to list anything explicitly). If any dependencies are missed by autodetection, that's a bug we can fix in the lxqt pkgconfig files (ie, they could be missing Requires: or Requires.private: keys)
I can start the formal review: 1. License: LGPL-2.1+ NOT ok, MUST use one listed on https://fedoraproject.org/wiki/Licensing probably want License: LGPLv2 2. MUST drop extraneous -devel dependency BuildRequires: pkgconfig(Qt5Xdg) >= 1.0.0 it is already list in the main package (no need to duplicate it here) 3. SHOULD not use %make_install macro (it's only compatible with very recent rpm versions, certainly not centos6), I'd recommend using instead: make install/fast DESTDIR=%{buildroot} -C build 4. SHOULD not omit translation, rm -f %{buildroot}/%{_datadir}/lxqt-qt5/translations/liblxqt/liblxqt_sr or at least provide justification why. 5. SHOULD track library soname, instead of %{_libdir}/liblxqt-qt5.so.* use %{_libdir}/liblxqt-qt5.so.0* (this will give you warning when incompatible library versions land when updating the package) I'd encourage you to consider adopting something close to helio's version, it does not suffer any of these issues.
Eugene, Rex, can we just start from my latest build ? Is just an evolution from the Eugene anyway, and them we settle this down for good. https://heliocastro.fedorapeople.org/lxqt/liblxqt-0.8.0-4.fc21.src.rpm
I don't care, but that's up to Eugene how to proceed for now. We can certainly merge additional necessary changes after the review is done.
(In reply to Helio Chissini de Castro from comment #6) > Eugene, Rex, can we just start from my latest build ? > Is just an evolution from the Eugene anyway, and them we settle this down > for good. > > https://heliocastro.fedorapeople.org/lxqt/liblxqt-0.8.0-4.fc21.src.rpm We can't start before review is done. And I agree with Rex - I can add your patches after.
ping, eugene, can you post updated .spec to comply with issues raised in comment #5 ?
(In reply to Rex Dieter from comment #9) > ping, eugene, can you post updated .spec to comply with issues raised in > comment #5 ? Oops... Sorry - I get lost in lxqt-related emails :-) 1. #3 - maybe - to make centos6-only workaround? 2. #4 - the same (this is rhel6-specific) 3. About Helio's patche - I need investigations - how they will works. Will try tomorrow. 4. What about I will not update release with "comment #5" fixes from 1 to 2?
The only items that are review blockers are MUST items (1,2), the others would be nice (and recommended), but not required to pass review.
(In reply to Rex Dieter from comment #11) > The only items that are review blockers are MUST items (1,2), the others > would be nice (and recommended), but not required to pass review. 1,2 was not replied because they are must. I was interesting in details. So, if you permit me _not_ update release - then I will tomorrow: * fix #1, #2, #5 * make centos6-workaround for #3, #5. * Helio's patches will be in next (or current) release. Need tryings. * release will be "1" Ok? PS. Because fine number :-)
It is good practice to increment Release and add a changelog entry for *every* change, particularly important in package reviews. See also: https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs p.s. I don't care about helio's patches for now, we can address that after the review.
(In reply to Rex Dieter from comment #5) > I can start the formal review: #1..5 fixed: Spec: https://tieugene.fedorapeople.org/rpms/liblxqt/liblxqt.spec SRPM: https://tieugene.fedorapeople.org/rpms/liblxqt/liblxqt-0.8.0-2.fc20.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8060403 (other distros haven't libqtxdg 1.0.0)
didn't address item 4, why? Nor satisfactorily address 3, instead of simplifying as I suggested, you made it more complicated by introducing a conditional. Either way, those were not blockers, the rest looks good, thanks. APPROVED
Now can you add me as a co-maintainer of the package ? I already did the ACL's requests and then i can push the patches
New Package SCM Request ======================= Package Name: liblxqt Short Description: Core LXQT library Upstream URL: http://www.lxde.org Owners: heliocastro tieugene rdieter Branches: f19 f20 f21 el6 epel7 InitialCC:
Git done (by process-git-requests).
imported, thanks.
Removing alias to allow general search for bugs.