Bug 523326 - Review Request: gnome-applet-window-picker - Window picker applet for GNOME
Review Request: gnome-applet-window-picker - Window picker applet for GNOME
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
: 451773 (view as bug list)
Depends On:
Blocks: FedoraMini/Mobility
  Show dependency treegraph
 
Reported: 2009-09-14 17:48 EDT by Michel Alexandre Salim
Modified: 2009-10-06 06:08 EDT (History)
4 users (show)

See Also:
Fixed In Version: 0.5.6-3.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-10-06 05:58:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
cwickert: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Michel Alexandre Salim 2009-09-14 17:48:29 EDT
Spec URL: http://salimma.fedorapeople.org/specs/netbook/gnome-applets-window-picker.spec
SRPM URL: http://salimma.fedorapeople.org/specs/netbook/gnome-applets-window-picker-0.5.6-2.fc12.src.rpm
Description:
A gnome-panel applet that displays open windows as icons on the panel,
and has integrated window title-bar functionality. Optimised for use
on netbook-size screens.
Comment 1 Michel Alexandre Salim 2009-09-14 17:51:32 EDT
*** Bug 451773 has been marked as a duplicate of this bug. ***
Comment 2 Michel Alexandre Salim 2009-09-14 17:54:20 EDT
Been running this version on my netbook since yesterday, with no problem so far.

Koji F-11 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1678712
Koji F-12 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1678683
Comment 3 Christoph Wickert 2009-09-14 18:41:25 EDT
I'd be happy to review this.

IMO the package name should be gnome-applet-window-picker (applet vs. applets) as it only contains a single applet. No need to make a new package now, stay tuned for the review.
Comment 4 Michel Alexandre Salim 2009-09-14 19:13:57 EDT
Thanks!

As for the naming, IMHO, my understanding is that gnome-applets-window-picker means "this is a related subpackage of gnome-applets, providing a window picker", rather than "this is a collection of gnome applets".

Thus someone who does "yum search gnome-applets" would see gnome-applets-window-picker, but would not see gnome-applet-window-picker. No precedent, though, it seems -- there have been other applets before but none have adopted the subpackage naming convention.
Comment 5 Christoph Wickert 2009-09-14 19:39:37 EDT
gnome-applets-window-picker would be correct if it was build from the same source as gnome-applets. But it's not, it's a package of it's own.

The gnome-applet case is covered by the naming guidelines, see
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28General.29

I doubt someone will search for "applets" but for "applet" instead and it will return: 
$ yum search gnome-applet
============================ Matched: gnome-applet ============================
gnome-applets.x86_64 : Small applications for the GNOME panel
gnome-applet-alarm-clock.x86_64 : Alarm clock for the GNOME panel
gnome-applet-bubblemon.x86_64 : Bubbling Load Monitoring Applet for the GNOME Panel
gnome-applet-cpufire.x86_64 : GNOME panel applet showing the CPU load as a fire
gnome-applet-globalmenu.x86_64 : GNOME panel applet of Global Menu
gnome-applet-grdc.x86_64 : Remote desktop client based on GTK+ and Gnome
gnome-applet-jalali-calendar.noarch : Jalali calendar panel applet for GNOME
gnome-applet-music.x86_64 : A GNOME panel applet to control various music players
gnome-applet-netspeed.x86_64 : GNOME applet that shows traffic on a network device
gnome-applet-sensors.i586 : Gnome panel applet for hardware sensors
gnome-applet-sensors.x86_64 : Gnome panel applet for hardware sensors
gnome-applet-sensors-devel.i586 : Development files for gnome-applet-sensors
gnome-applet-sensors-devel.x86_64 : Development files for gnome-applet-sensors
gnome-applet-sshmenu.noarch : GNOME panel applet to organize SSH connection information in a menu
gnome-applet-timer.x86_64 : A countdown timer applet for the GNOME panel
gnome-applet-vm.x86_64 : Simple virtual domains monitor which embeds itself in the GNOME panel

So "applet" is in line with the rest of the packages and IMO we should stick with that.
Comment 6 Michel Alexandre Salim 2009-09-14 20:16:21 EDT
Ah, duh, and that's why I did not find any. OK, I'll rename it in the final version. Probably not worth creating a new SRPM though.
Comment 7 Christoph Wickert 2009-09-15 19:44:57 EDT
I got a 404 on the spec and realized you already changed it. Fine.

REVIEW FOR 
e5168e16cd03c7f4bd3d01358a098056  gnome-applets-window-picker-0.5.6-2.fc12.src.rpm


FIX - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/gnome-applets-window-picker-*
gnome-applets-window-picker.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 3)
gnome-applets-window-picker-debuginfo.x86_64: E: description-line-too-long This package provides debug information for package gnome-applets-window-picker.
3 packages and 0 specfiles checked; 1 errors, 1 warnings.

there is nothing we can do about the latter, but please fix the first one, although it's just cosmetic.

OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines: GPLv3
OK - MUST: License field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: sources match the upstream source by MD5 aeda4b063a0233920b0186bca4495fef
OK - MUST: successfully compiles and builds into binary rpms on x86_64
OK - MUST: no ExcludeArch.
OK - MUST: all build dependencies are listed in BuildRequires.
OK - MUST: handles locales properly with %find_lang
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT
OK - MUST: consistently uses macros
OK - MUST: package contains code, or permissable content
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package.
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: packages does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock.
OK - SHOULD: compiles and builds into binary rpms on all supported architectures.
OK - SHOULD: functions as described.
OK - SHOULD: scriptlets are sane
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - compiler flags honored
OK - debuginfo complete
OK - Provides: are sane.


Issues:
Fix - Make is not verbose:
make[2]: Entering directory `/home/chris/fedora/rpmbuild/BUILD/window-picker-applet-0.5.6/src'
  CC     task-item.o
  CC     applet.o
  CC     task-list.o
  CC     task-title.o
  CCLD   window-picker-applet

Use make %{?_smp_mflags} V=1

Add Requires: gnome-panel
because the package only requires gnome-panel-libs currently.

IMO you should add ChangeLog to %doc


Rename the package, fix these and consider this package APPROVED.
Comment 8 Christoph Wickert 2009-09-15 21:02:59 EDT
(In reply to comment #7)
> Add Requires: gnome-panel

Uhh, the guidelines have changed recently. This should be a versioned dependency as described in https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires
Comment 9 Christoph Wickert 2009-09-16 20:48:35 EDT
Pls use %global instead of %define
https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define
Comment 10 Michel Alexandre Salim 2009-09-19 21:37:50 EDT
Will make the final updates for the CVS import, thanks for noting them. Actually, I'll check if the SONAME for gnome-panel changes from one Fedora release to another; we'd probably be OK with just relying on the automatic requires: -- not on my Fedora machine now, so I can't test, but I'll verify this (note to self).

New Package CVS Request
=======================
Package Name: gnome-applet-window-picker
Short Description:  Window picker applet for GNOME
Owners: salimma
Branches: F-10 F-11 EL-5
InitialCC:
Comment 11 Jason Tibbitts 2009-09-21 21:35:27 EDT
CVS done.
Comment 12 Fedora Update System 2009-09-23 16:55:56 EDT
gnome-applet-window-picker-0.5.6-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/gnome-applet-window-picker-0.5.6-2.fc11
Comment 13 Fedora Update System 2009-09-23 16:56:02 EDT
gnome-applet-window-picker-0.5.6-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gnome-applet-window-picker-0.5.6-2.fc10
Comment 14 Christoph Wickert 2009-09-23 19:51:34 EDT
Michel, AFAICS you did not fix any of the issues I pointed out in comment #7:
- rpmlint is not clean
- make is not verbose
- package doesn't require gnome-panel
- ChhangeLog missing from %doc

What's going on here?
Comment 15 Michel Alexandre Salim 2009-09-23 22:49:05 EDT
(In reply to comment #14)
> Michel, AFAICS you did not fix any of the issues I pointed out in comment #7:
> - rpmlint is not clean

Not sure what you mean; it is clean here:
$ rpmlint /home/michel/Download/gnome-applet-window-picker-0.5.6-2.fc12.i686.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint gnome-applet-window-picker
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint /home/michel/Download/gnome-applet-window-picker-debuginfo-0.5.6-2.fc12.i686.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

> - make is not verbose
> - package doesn't require gnome-panel
> - ChangeLog missing from %doc

You're right on these; I misremembered having fixed all the TODO list. Updated build will follow.
Comment 16 Michel Alexandre Salim 2009-09-23 22:57:15 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > Add Requires: gnome-panel
> 
> Uhh, the guidelines have changed recently. This should be a versioned
> dependency as described in
> https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires  

Not sure that is the correct reading. The guideline uses that versioned dependency as an example, and it actually recommended removing the version requirement, once the oldest supported Fedora release exceeds that requirement.

In this case, since there is a SONAME dependency that will pull in the correct version of gnome-panel-libs, the correct version of gnome-panel would be pulled in as well (because of the same SONAME dependency between gnome-panel and libpanel-applet)

Actually, this exposes a gnome-panel bug: surely gnome-panel should require the same %{version}-%{release} of gnome-panel-libs...
Comment 17 Christoph Wickert 2009-09-24 04:49:40 EDT
(In reply to comment #15)
> (In reply to comment #14)
> > Michel, AFAICS you did not fix any of the issues I pointed out in comment #7:
> > - rpmlint is not clean
> 
> Not sure what you mean; it is clean here:

Please see comment # 7 or run rpmlint on the srpm:
$ rpmlint Downloads/gnome-applet-window-picker-0.5.6-2.fc10.src.rpm 
gnome-applet-window-picker.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 3)
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

(In reply to comment #16)
> Not sure that is the correct reading. The guideline uses that versioned
> dependency as an example, and it actually recommended removing the version
> requirement, once the oldest supported Fedora release exceeds that requirement.

I read this different: Since non of the downsides apply to this package, a versioned dep should be used here. However...

> In this case, since there is a SONAME dependency that will pull in the correct
> version of gnome-panel-libs, the correct version of gnome-panel would be pulled
> in as well (because of the same SONAME dependency between gnome-panel and
> libpanel-applet)

... I fully agree with you in this special case.
Comment 18 Fedora Update System 2009-09-24 14:42:39 EDT
gnome-applet-window-picker-0.5.6-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gnome-applet-window-picker-0.5.6-3.fc10
Comment 19 Fedora Update System 2009-09-24 14:42:45 EDT
gnome-applet-window-picker-0.5.6-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/gnome-applet-window-picker-0.5.6-3.fc11
Comment 20 Michel Alexandre Salim 2009-09-24 21:35:10 EDT
(In reply to comment #17)
> 
> Please see comment # 7 or run rpmlint on the srpm:
> $ rpmlint Downloads/gnome-applet-window-picker-0.5.6-2.fc10.src.rpm 
> gnome-applet-window-picker.src: W: mixed-use-of-spaces-and-tabs (spaces: line
> 1, tab: line 3)
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
This is fixed in the new spec. Thanks!
Comment 21 Fedora Update System 2009-09-29 10:21:44 EDT
gnome-applet-window-picker-0.5.6-3.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gnome-applet-window-picker'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-9965
Comment 22 Fedora Update System 2009-09-29 10:31:25 EDT
gnome-applet-window-picker-0.5.6-3.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gnome-applet-window-picker'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10032
Comment 23 Fedora Update System 2009-10-06 05:58:35 EDT
gnome-applet-window-picker-0.5.6-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Fedora Update System 2009-10-06 06:08:02 EDT
gnome-applet-window-picker-0.5.6-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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