Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1159873 (lxqt-panel) - Review Request: lxqt-panel - Main panel bar for LXQt desktop suite
Summary: Review Request: lxqt-panel - Main panel bar for LXQt desktop suite
Keywords:
Status: CLOSED RAWHIDE
Alias: lxqt-panel
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1157402 liblxqt-mount
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-11-03 14:01 UTC by Helio Chissini de Castro
Modified: 2015-11-12 01:44 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-11-20 13:19:49 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Helio Chissini de Castro 2014-11-03 14:01:23 UTC
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

Comment 1 Mamoru TASAKA 2014-11-04 14:28:16 UTC
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.

Comment 2 Mamoru TASAKA 2014-11-04 14:31:46 UTC
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:"

Comment 3 Helio Chissini de Castro 2014-11-04 14:41:14 UTC
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

Comment 4 Rex Dieter 2014-11-07 19:26:14 UTC
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

Comment 6 Rex Dieter 2014-11-10 15:33:05 UTC
3 is apparently still not fixed, %files only has:
%{_libdir}/lxqt-panel/*.so

Comment 8 Rex Dieter 2014-11-10 17:31:04 UTC
Good, thanks, APPROVED

Comment 9 Helio Chissini de Castro 2014-11-12 15:12:52 UTC
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

Comment 10 Gwyn Ciesla 2014-11-13 19:43:03 UTC
Git done (by process-git-requests).

Comment 11 Rex Dieter 2014-11-20 13:19:49 UTC
imported


Note You need to log in before you can comment on or make changes to this bug.