Bug 896193 (plank)

Summary: Review Request: plank - A port of docky to Vala
Product: [Fedora] Fedora Reporter: Wesley Hearn <whearn>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: ad2clark, echevemaster, eldermarco, i, ricotz, whearn
Target Milestone: ---Flags: i: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: plank-0.5.0-4.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-02-26 08:59:37 EST Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On: 988667, 1017499    
Bug Blocks: 1068850    

Description Wesley Hearn 2013-01-16 15:12:12 EST
Spec URL: http://jknife.fedorapeople.org/review/plank.spec
SRPM URL: http://jknife.fedorapeople.org/review/plank-0.0-1.20130116bzr722.el6.src.rpm
Description: A very simple dock written in Vala.
Fedora Account System Username: jknife
Comment 1 Wesley Hearn 2013-01-16 15:50:46 EST
Upstream bug regarding the license file not being included https://bugs.launchpad.net/plank/+bug/1100448
Comment 2 Mario Bl├Ąttermann 2013-01-17 14:55:13 EST
Some initial issues:

It is not needed to extract the configure macro. Just put autogen.sh before running %configure.



%{_datadir}/icons/gnome/16x16/apps/%{name}.svg
%{_datadir}/icons/gnome/22x22/apps/%{name}.svg
%{_datadir}/icons/gnome/24x24/apps/%{name}.svg
%{_datadir}/icons/gnome/32x32/apps/%{name}.svg
%{_datadir}/icons/gnome/48x48/apps/%{name}.svg
%{_datadir}/icons/gnome/64x64/apps/%{name}.svg
%{_datadir}/icons/gnome/128x128/apps/%{name}.svg
%{_datadir}/icons/hicolor/16x16/apps/%{name}.svg
%{_datadir}/icons/hicolor/22x22/apps/%{name}.svg
%{_datadir}/icons/hicolor/24x24/apps/%{name}.svg
%{_datadir}/icons/hicolor/32x32/apps/%{name}.svg
%{_datadir}/icons/hicolor/48x48/apps/%{name}.svg
%{_datadir}/icons/hicolor/64x64/apps/%{name}.svg
%{_datadir}/icons/hicolor/128x128/apps/%{name}.svg

You could shrink this as follows:

%{_datadir}/icons/gnome/*/apps/%{name}.svg
%{_datadir}/icons/hicolor/*/apps/%{name}.svg



BuildRequires:  desktop-file-utils

This is fine, but the desktop file needs to be installed explicitely or at least validated:
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database

The latter is probably not applicable because the desktop file doesn't introduce new MIME types.



%clean
rm -rf %{buildroot}

Drop the %clean section. It is only needed for EPEL 5 packages.
Comment 3 Wesley Hearn 2013-01-18 11:46:02 EST
Updated:
SPEC: http://jknife.fedorapeople.org/review/plank.spec
SRCRPM: http://jknife.fedorapeople.org/review/plank-0.0-1.20130117bzr723.fc18.src.rpm

Initial items on your list are done, as for the .desktop file I was already calling desktop-file-validate.
Comment 5 Eduardo Echeverria 2013-01-24 01:59:21 EST
Hi Wesley 

- This is a snapshot and pre-release package, please follow the recommendations are explained on [1] and [2]
Where 
Release Tag for Pre-Release Packages: 0.%{X}.%{alphatag}
Where %{X} =  is the release number increment 
and 
%{alphatag} = is the string that came from the version (20130121bzr731)

- In this line:
cp -r %{buildroot}%{_datadir}/icons/hicolor/ %{buildroot}%{_datadir}/icons/gnome
Actually the icons are not broken, simply use proper scriptlets [2]

- the url should be [4] not http://wiki.go-docky.com/index.php

- This package builds in gtk2 and gtk3 with their respectives dependencies, take a look in [5]

- Please note that upstream accepts no Fedora related bugs [6]

- Please take a look at your rpmlint output

rpmlint -vi plank-0.2.0.731-1.20130121.fc17.x86_64.rpm 
plank.x86_64: W: shared-lib-calls-exit /usr/lib64/libplank.so.0.0.0 exit@GLIBC_2.2.5
This library package calls exit() or _exit(), probably in a non-fork()
context. Doing so from a library is strongly discouraged - when a library
function calls exit(), it prevents the calling program from handling the
error, reporting it to the user, closing files properly, and cleaning up any
state that the program has. It is preferred for the library to return an
actual error code and let the calling program decide how to handle the
situation.

- checking for DBUSMENU... no
As an aside, one of the unfulfilled dependencies of this package is libdebusmenu, although I do not understand why it was not included in Fedora,  being a approved package , I recommend that you take a look at this link [7]


[1] http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages
[2] http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages
[3] http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
[4] https://launchpad.net/plank
[5] http://wiki.go-docky.com/index.php?title=Plank:Installing
[6] http://wiki.go-docky.com/index.php?title=Plank:Introduction#Reporting_Bugs
[7] bug 842509

Btw, is available rev 734

Regards
Comment 6 Wesley Hearn 2013-01-24 11:26:31 EST
- Fixed
- Fixed
- Fixed
- No, that page is outdated, They removed GTK2 support in June of last year(Changelog entry)
- Asking about this
- Will file a bug
- It is not essential to plank working. 

SPEC: http://jknife.fedorapeople.org/review/plank.spec
SRPM: http://jknife.fedorapeople.org/review/plank-0.2.0.734-0.1.20130124bzr.fc18.src.rpm
Comment 7 Robert Dyer 2013-01-29 16:30:29 EST
Upstream now accepts Fedora bugs.

http://wiki.go-docky.com/index.php?title=Plank:Introduction#Reporting_Bugs
Comment 8 Robert Dyer 2013-01-29 16:31:26 EST
The upstream installation instructions no longer reference GTK2.

http://wiki.go-docky.com/index.php?title=Plank:Installing
Comment 9 Eduardo Echeverria 2013-01-30 02:05:48 EST
Wesley:

Sorry for delay, I had a busy few days

(In reply to comment #7)
> Upstream now accepts Fedora bugs.
> 
> http://wiki.go-docky.com/index.php?title=Plank:Introduction#Reporting_Bugs

Are good news !!

About versioning. 
- Current development version is 0.2.0
- rev is in this case 734 
therefore, the versioning should be: 0.2.0-0.1.20130124bzr734 
and the release number increment every time you make changes in the spec: 
e.g.
0.2.0-0.2.20130124bzr734  
0.2.0-0.3.20130124bzr734 
and so on
in the spec: 
%global         bzrid 734
%global         checkout 20130124bzr%{bzrid}
Name:           plank
Version:        0.2.0
Release:        0.1.%{checkout}%{?dist}
Source0:        plank-%{version}.%{bzrid}.tar.xz

About @shared-lib-calls-exit 
this is no review blocker,  .But it is worth to inform the upstream about that. (and fix it soon)

Let's review

rpmlint plank plank-devel plank-debuginfo
plank.x86_64: W: spelling-error Summary(en_US) docky -> dock, docks, dorky
plank.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libplank.so.0.0.0 /lib64/libgthread-2.0.so.0
plank.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libplank.so.0.0.0 /lib64/librt.so.1
plank.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libplank.so.0.0.0 /lib64/libatk-1.0.so.0
plank.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libplank.so.0.0.0 /lib64/libcairo-gobject.so.2
plank.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libplank.so.0.0.0 /lib64/libpthread.so.0

See http://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency

objdump -p libplank.so.0.0.0 | grep NEEDED
  NEEDED               libm.so.6
  NEEDED               libgthread-2.0.so.0
  NEEDED               librt.so.1 *
  NEEDED               libbamf3.so.0
  NEEDED               libwnck-3.so.0
  NEEDED               libgtk-3.so.0
  NEEDED               libgdk-3.so.0
  NEEDED               libatk-1.0.so.0 * 
  NEEDED               libgio-2.0.so.0
  NEEDED               libpangocairo-1.0.so.0
  NEEDED               libgdk_pixbuf-2.0.so.0
  NEEDED               libcairo-gobject.so.2 *
  NEEDED               libpango-1.0.so.0
  NEEDED               libcairo.so.2
  NEEDED               libgee.so.2
  NEEDED               libgobject-2.0.so.0
  NEEDED               libglib-2.0.so.0
  NEEDED               libX11.so.6
  NEEDED               libpthread.so.0 *
  NEEDED               libc.so.6

- (nonblocker) You might mention to upstream the various warnings

Drawing/DockSurface.vala:366.32-366.51: warning: GLib.Thread.create has been deprecated since 2.32. Use new Thread<T> ()
Drawing/DockSurface.vala:375.10-375.29: warning: GLib.Thread.create has been deprecated since 2.32. Use new Thread<T> ()
Drawing/DockSurface.vala:492.32-492.51: warning: GLib.Thread.create has been deprecated since 2.32. Use new Thread<T> ()
Drawing/DockSurface.vala:515.10-515.29: warning: GLib.Thread.create has been deprecated since 2.32. Use new Thread<T> ()
Drawing/DrawingService.vala:54.49-54.77: warning: GLib.FILE_ATTRIBUTE_THUMBNAIL_PATH has been deprecated since vala-0.16. Use FileAttribute.THUMBNAIL_PATH
Factories/AbstractMain.vala:271.4-271.15: warning: `null' incompatible with return type `string[]`
return null;
^^^^^^^^^^^^
Items/ApplicationDockItem.vala:442.52-442.87: warning: GLib.FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE has been deprecated since vala-0.16. Use FileAttribute.STANDARD_CONTENT_TYPE
Items/FileDockItem.vala:294.52-294.79: warning: GLib.FILE_ATTRIBUTE_STANDARD_NAME has been deprecated since vala-0.16. Use FileAttribute.STANDARD_NAME
Items/FileDockItem.vala:295.8-295.40: warning: GLib.FILE_ATTRIBUTE_STANDARD_IS_HIDDEN has been deprecated since vala-0.16. Use FileAttribute.STANDARD_IS_HIDDEN
Items/FileDockItem.vala:296.8-296.37: warning: GLib.FILE_ATTRIBUTE_ACCESS_CAN_READ has been deprecated since vala-0.16. Use FileAttribute.ACCESS_CAN_READ
DockItems.vala:388.42-388.69: warning: GLib.FILE_ATTRIBUTE_STANDARD_NAME has been deprecated since vala-0.16. Use FileAttribute.STANDARD_NAME
DockItems.vala:388.79-388.111: warning: GLib.FILE_ATTRIBUTE_STANDARD_IS_HIDDEN has been deprecated since vala-0.16. Use FileAttribute.STANDARD_IS_HIDDEN
DockItems.vala:224.77-224.104: warning: GLib.FILE_ATTRIBUTE_STANDARD_NAME has been deprecated since vala-0.16. Use FileAttribute.STANDARD_NAME
DockItems.vala:224.114-224.146: warning: GLib.FILE_ATTRIBUTE_STANDARD_IS_HIDDEN has been deprecated since vala-0.16. Use FileAttribute.STANDARD_IS_HIDDEN
Services/WindowControl.vala:124.3-124.41: warning: method `Plank.Services.Windows.WindowControl.has_minimized_window' never used
public static bool has_minimized_window (Bamf.Application app)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Services/WindowControl.vala:166.3-166.50: warning: method `Plank.Services.Windows.WindowControl.get_windows' never used
public static ArrayList<Bamf.Window> get_windows (Bamf.Application app)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Services/WindowControl.vala:237.3-237.26: warning: method `Plank.Services.Windows.WindowControl.focus' never used
public static void focus (Bamf.Application app)
^^^^^^^^^^^^^^^^^^^^^^^^
Services/WindowControl.vala:294.3-294.29: warning: method `Plank.Services.Windows.WindowControl.minimize' never used
public static void minimize (Bamf.Application app)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Services/WindowControl.vala:303.3-303.28: warning: method `Plank.Services.Windows.WindowControl.restore' never used
public static void restore (Bamf.Application app)
^^^^^^^^^^^^^^^^^^^^^^^^^^
Services/WindowControl.vala:479.3-479.47: warning: method `Plank.Services.Windows.WindowControl.get_easy_geometry' never used
public static Gdk.Rectangle get_easy_geometry (Wnck.Window w)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Source file missing in debuginfo package:

cpio: plank-0.2.0.734/lib/AbstractMain.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/AnimatedRenderer.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/ApplicationDockItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Color.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/CompositedWindow.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DockItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DockItemPreferences.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DockSurface.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DockTheme.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DockWindow.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/DrawingService.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Factory.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/FileDockItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/HoverTheme.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/HoverWindow.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/ItemFactory.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Logger.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Matcher.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Paths.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/PlankDockItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/PoofWindow.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Preferences.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/System.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Theme.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/TitledSeparatorMenuItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/TransientDockItem.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/Unity.c: Cannot stat: No such file or directory
cpio: plank-0.2.0.734/lib/WindowControl.c: Cannot stat: No such file or directory

Consider fix these points deal with upstream,  and I'll do the formal review

Kind Regards
Eduardo
Comment 10 Eduardo Echeverria 2013-02-08 02:36:19 EST
Ping... Any update here?
Comment 11 Wesley Hearn 2013-02-08 08:54:03 EST
Hey, sorry. I have been busy with work, once I get some free time(hopefully this weekend or next week) I will work on this some more. I asked about libdbusmenu and it will have to be re-reviewed because it has taken so long. So I guess the goal is to pick that up and dust it off, get it approved and in Fedora then carry on with this one.
Comment 12 Christopher Meng 2013-07-25 05:34:35 EDT
Hi,

I just want to package it, will you continue?
Comment 13 Aaron Clark 2013-08-07 21:38:19 EDT
I am interested in this package as well.  I actually packaged it locally back in February but didn't take it any further due to the lack of a proper release.  I believe I needed a newer version of bamf-daemon when I tried to get it working back then.  It has been rock solid for me in the intervening period on Fedora 17, but I have not upgraded yet so have not done any testing on the more recent releases other than mock builds.

Here are my specs and srpms, hope they help:

http://fedorapeople.org/~ophidian/plank.spec
http://fedorapeople.org/~ophidian/plank-0.3.0-1.fc17.src.rpm

http://fedorapeople.org/~ophidian/bamf.spec
http://fedorapeople.org/~ophidian/bamf-0.2.126-1.fc17.src.rpm


Based on a quick rpmlint -vi, mine may not be any better than picking up Wesley's version and updating it:
$ rpmlint -vi plank-0.3.0-1.fc18.x86_64.rpm
plank.x86_64: I: checking
plank.x86_64: W: spelling-error %description -l en_US docky -> dock, docks, dorky
The value of this tag appears to be misspelled. Please double-check.

plank.x86_64: I: checking-url http://wiki.go-docky.com/index.php?title=Plank:Introduction (timeout 10 seconds)
plank.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/plank ['/usr/lib64']
The binary or shared library defines `RPATH'. Usually this is a bad thing
because it hardcodes the path to search libraries and so makes it difficult to
move libraries around.  Most likely you will find a Makefile with a line like:
gcc test.o -o test -Wl,--rpath.  Also, sometimes configure scripts provide a
--disable-rpath flag to avoid this.

plank.x86_64: E: zero-length /usr/share/doc/plank-0.3.0/README
plank.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libplank.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

plank.x86_64: E: postin-without-ldconfig /usr/lib64/libplank.so.0.0.0
This package contains a library and its %post scriptlet doesn't call ldconfig.

plank.x86_64: E: postun-without-ldconfig /usr/lib64/libplank.so.0.0.0
This package contains a library and its %postun doesn't call ldconfig.

1 packages and 0 specfiles checked; 4 errors, 2 warnings.
Comment 14 Christopher Meng 2013-08-07 21:54:27 EDT
I'll leave needinfo again to the reporter.
Comment 15 Wesley Hearn 2013-08-08 10:06:02 EDT
Hey sorry about that.

New Spec: http://jknife.fedorapeople.org/review/plank.spec
New SRPM: http://jknife.fedorapeople.org/review/plank-0.3.0-1.fc19.src.rpm
Comment 16 Christopher Meng 2013-08-27 13:44:32 EDT
1. Source0 Invalid now:

Actually URL is:

https://launchpad.net/plank/1.0/0.3.0/+download/plank-0.3.0.tar.xz

So you need to modify it to:

https://launchpad.net/%{name}/1.0/%{version}/+download/%{name}-%{version}.tar.xz

2. log:

checking for BAMF... no
checking for GEE_0_8... no
checking for DBUSMENU... yes

So you've missed bamf as BR.

======================

plank 0.3.0
    Prefix......................:  /usr
    Vala Compiler...............:  /usr/bin/valac
    Vala Flags..................:  --target-glib=2.32 --define HAVE_DBUSMENU
    C Compiler Flags............:  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables
    Use gee-0.8.................:  no
    Dbusmenu support............:  yes
    Headless Tests..............:  no
    Documentation...............:  no

-: This package requires valadoc, I've packaged it but still need upstream to change something, so I block this bug. 

But for the headless tests, I think you should enable them.


3. devel package need %{?_isa} tag.

4. %{configure}

should be:

%configure

5. %doc COPYING is not enough:

%doc AUTHORS ChangeLog COPYING

6. Don't leave too many blank lines, for example, 

%package devel

Summary:        Development files for %{name}
Group:          Development/Libraries
Requires:       %{name} = %{version}-%{release}
Requires:       vala-tools

should be

%package devel
Summary:        Development files for %{name}
Group:          Development/Libraries
Requires:       %{name} = %{version}-%{release}
Requires:       vala-tools

and the description is not good, change it to something like:

The %{name}-devel package contains libraries and header files for
developing applications that use %{name}.

7. This package use hidden build, so you can't know the flags/status when building it. Please append V=1 to make %{?_smp_mflags} like this:

make %{?_smp_mflags} V=1

to make sure build is not hidden.
Comment 17 Wesley Hearn 2013-08-27 17:04:50 EDT
1) Fixed

2)
 - I did not miss BAMF, it is checking for BAMF 0.4 now instead of 3. It still builds just find with 3.
 - Added tests trying to figure out how to check them
 - Ok on valadoc
 - I have also updated which libgee it looks for. So it is now building against the correct one.

3) Done

4) Changed

5) Done

6) Changed

7) Done

Once I figure out the tests I will push up a updated spec/srpm.
Comment 18 Christopher Meng 2013-08-27 20:56:20 EDT
How did you try valadoc?
Comment 19 Wesley Hearn 2013-08-28 10:19:03 EDT
I didn't, I was just acking that it is needed.
Comment 20 Rico Tzschichholz 2013-10-09 10:19:21 EDT
valadoc is not a hard-requirement. You can also use the prebuilt docs included in the tarball if needed.

Using gee-0.8 is recommended and also the current standard for GNOME apps. Although using gee-0.6 (aka gee-1.0) works too.
Comment 21 Rico Tzschichholz 2013-10-09 10:21:28 EDT
Doing a secondary check for BAMF >= 0.4 is just wanted to follow an API change to previous versions.
Comment 22 Christopher Meng 2013-10-09 23:12:49 EDT
(In reply to Rico Tzschichholz from comment #21)
> Doing a secondary check for BAMF >= 0.4 is just wanted to follow an API
> change to previous versions.

I know, but we are trying to build from source for everything.

Curretnly valadoc is broken and is being fixed, and we can temporarily disable that option, but after it can be used we need to enable the option again.

@Wesley, for the libgee issue, I think you should consider about it...
Comment 23 Christopher Meng 2013-10-09 23:41:57 EDT
(In reply to Rico Tzschichholz from comment #21)
> Doing a secondary check for BAMF >= 0.4 is just wanted to follow an API
> change to previous versions.

HI Rico, Fedora has 2 bamf packages, one is built on GTK2, the other is built on GTK3, and from comment 8 we can see that gtk2 is no longer recommended, so it's better to switch to gtk3. And then I think we should use bamf3.

But the first thing need to be solved seems changed to bamf update...

The latest version is 0.5, I just opened a bug report and let is block this.
Comment 24 Rico Tzschichholz 2013-10-10 02:07:43 EDT
(In reply to Christopher Meng from comment #23)
> (In reply to Rico Tzschichholz from comment #21)
> > Doing a secondary check for BAMF >= 0.4 is just wanted to follow an API
> > change to previous versions.
> 
> HI Rico, Fedora has 2 bamf packages, one is built on GTK2, the other is
> built on GTK3, and from comment 8 we can see that gtk2 is no longer
> recommended, so it's better to switch to gtk3. And then I think we should
> use bamf3.
> 
> But the first thing need to be solved seems changed to bamf update...
> 
> The latest version is 0.5, I just opened a bug report and let is block this.

I am not referring to gtk2 in any case. There is no support from plank's side anyway. The minimum requirement is libbamf3 >= 0.2.92 and newer versions are *optional*. Of course having some newer libbamf3 will bring in wanted bug fixes.
Comment 25 Wesley Hearn 2014-01-14 22:34:00 EST
Here is the latest:
SPEC: http://jknife.fedorapeople.org/review/plank.spec
SRPM: http://jknife.fedorapeople.org/review/plank-0.5.0-1.fc20.src.rpm

As for BAMF4 as Rico said we do not need it, BAMF3 is enough.
For valadoc a bug can be opened afterwards and I will add the docs to the rpm.
Comment 26 Christopher Meng 2014-02-10 20:05:50 EST
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "LGPL (v2.1 or later)", "GPL (v2 or later)", "GPL (v3 or later)",
     "Unknown or generated". 34 files have unknown license. Detailed output of
     licensecheck:

GPL (v2 or later)
-----------------
plank-0.5.0/build-aux/ltmain.sh

GPL (v3 or later)
-----------------
plank-0.5.0/data/apport/source_plank.py
plank-0.5.0/lib/DockController.vala
plank-0.5.0/lib/DockPreferences.vala
plank-0.5.0/lib/DockRenderer.vala
plank-0.5.0/lib/DragManager.vala
plank-0.5.0/lib/Drawing/AnimatedRenderer.vala
plank-0.5.0/lib/Drawing/Color.vala
plank-0.5.0/lib/Drawing/DockSurface.vala
plank-0.5.0/lib/Drawing/DockTheme.vala
plank-0.5.0/lib/Drawing/DrawingService.vala
plank-0.5.0/lib/Drawing/Theme.vala
plank-0.5.0/lib/Factories/AbstractMain.vala
plank-0.5.0/lib/Factories/Factory.vala
plank-0.5.0/lib/Factories/ItemFactory.vala
plank-0.5.0/lib/HideManager.vala
plank-0.5.0/lib/Items/ApplicationDockItem.vala
plank-0.5.0/lib/Items/ApplicationDockItemProvider.vala
plank-0.5.0/lib/Items/DefaultApplicationDockItemProvider.vala
plank-0.5.0/lib/Items/DockItem.vala
plank-0.5.0/lib/Items/DockItemPreferences.vala
plank-0.5.0/lib/Items/DockItemProvider.vala
plank-0.5.0/lib/Items/FileDockItem.vala
plank-0.5.0/lib/Items/PlankDockItem.vala
plank-0.5.0/lib/Items/TransientDockItem.vala
plank-0.5.0/lib/PositionManager.vala
plank-0.5.0/lib/Services/Logger.vala
plank-0.5.0/lib/Services/Matcher.vala
plank-0.5.0/lib/Services/Paths.vala
plank-0.5.0/lib/Services/Preferences.vala
plank-0.5.0/lib/Services/System.vala
plank-0.5.0/lib/Services/WindowControl.vala
plank-0.5.0/lib/Widgets/CompositedWindow.vala
plank-0.5.0/lib/Widgets/DockWindow.vala
plank-0.5.0/lib/Widgets/HoverWindow.vala
plank-0.5.0/lib/Widgets/PoofWindow.vala
plank-0.5.0/src/PlankMain.vala
plank-0.5.0/tests/Drawing.vala
plank-0.5.0/tests/Items.vala
plank-0.5.0/tests/Main.vala
plank-0.5.0/tests/Preferences.vala
plank-0.5.0/tests/Widgets.vala
plank-0.5.0/tests/test-config.h

LGPL (v2.1 or later)
--------------------
plank-0.5.0/lib/Widgets/TitledSeparatorMenuItem.vala

Unknown or generated
--------------------
plank-0.5.0/docs/c-doc/ccomments/AbstractMain.c
plank-0.5.0/docs/c-doc/ccomments/AnimatedRenderer.c
plank-0.5.0/docs/c-doc/ccomments/ApplicationDockItem.c
plank-0.5.0/docs/c-doc/ccomments/ApplicationDockItemProvider.c
plank-0.5.0/docs/c-doc/ccomments/Color.c
plank-0.5.0/docs/c-doc/ccomments/CompositedWindow.c
plank-0.5.0/docs/c-doc/ccomments/DefaultApplicationDockItemProvider.c
plank-0.5.0/docs/c-doc/ccomments/DockController.c
plank-0.5.0/docs/c-doc/ccomments/DockItem.c
plank-0.5.0/docs/c-doc/ccomments/DockItemPreferences.c
plank-0.5.0/docs/c-doc/ccomments/DockItemProvider.c
plank-0.5.0/docs/c-doc/ccomments/DockPreferences.c
plank-0.5.0/docs/c-doc/ccomments/DockRenderer.c
plank-0.5.0/docs/c-doc/ccomments/DockSurface.c
plank-0.5.0/docs/c-doc/ccomments/DockTheme.c
plank-0.5.0/docs/c-doc/ccomments/DockWindow.c
plank-0.5.0/docs/c-doc/ccomments/DragManager.c
plank-0.5.0/docs/c-doc/ccomments/DrawingService.c
plank-0.5.0/docs/c-doc/ccomments/Factory.c
plank-0.5.0/docs/c-doc/ccomments/FileDockItem.c
plank-0.5.0/docs/c-doc/ccomments/HideManager.c
plank-0.5.0/docs/c-doc/ccomments/HoverWindow.c
plank-0.5.0/docs/c-doc/ccomments/ItemFactory.c
plank-0.5.0/docs/c-doc/ccomments/Logger.c
plank-0.5.0/docs/c-doc/ccomments/Paths.c
plank-0.5.0/docs/c-doc/ccomments/PlankDockItem.c
plank-0.5.0/docs/c-doc/ccomments/PoofWindow.c
plank-0.5.0/docs/c-doc/ccomments/PositionManager.c
plank-0.5.0/docs/c-doc/ccomments/Preferences.c
plank-0.5.0/docs/c-doc/ccomments/System.c
plank-0.5.0/docs/c-doc/ccomments/Theme.c
plank-0.5.0/docs/c-doc/ccomments/TitledSeparatorMenuItem.c
plank-0.5.0/docs/c-doc/ccomments/TransientDockItem.c
plank-0.5.0/lib/abicheck.sh

[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in plank
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 143360 bytes in 3 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
     file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: plank-0.5.0-1.fc21.i686.rpm
          plank-devel-0.5.0-1.fc21.i686.rpm
          plank-0.5.0-1.fc21.src.rpm
plank.i686: W: spelling-error Summary(en_US) docky -> dock, docks, dorky
plank-devel.i686: W: no-documentation
plank.src: W: spelling-error Summary(en_US) docky -> dock, docks, dorky
3 packages and 0 specfiles checked; 0 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint plank plank-devel
plank.i686: W: spelling-error Summary(en_US) docky -> dock, docks, dorky
plank.i686: W: unused-direct-shlib-dependency /usr/lib/libplank.so.0.0.0 /lib/libpthread.so.0
plank-devel.i686: W: no-documentation
2 packages and 0 specfiles checked; 0 errors, 3 warnings.
# echo 'rpmlint-done:'



Requires
--------
plank (rpmlib, GLIBC filtered):
    /bin/sh
    bamf-daemon
    libX11.so.6
    libatk-1.0.so.0
    libbamf3.so.0
    libc.so.6
    libcairo-gobject.so.2
    libcairo.so.2
    libdbusmenu-glib.so.4
    libdbusmenu-gtk3.so.4
    libgdk-3.so.0
    libgdk_pixbuf-2.0.so.0
    libgee-0.8.so.2
    libgio-2.0.so.0
    libglib-2.0.so.0
    libgobject-2.0.so.0
    libgthread-2.0.so.0
    libgtk-3.so.0
    libpango-1.0.so.0
    libpangocairo-1.0.so.0
    libplank.so.0
    libpthread.so.0
    libwnck-3.so.0
    rtld(GNU_HASH)

plank-devel (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    /usr/bin/pkg-config
    libplank.so.0
    pkgconfig(cairo)
    pkgconfig(gdk-3.0)
    pkgconfig(gdk-pixbuf-2.0)
    pkgconfig(gee-0.8)
    pkgconfig(gio-2.0)
    pkgconfig(glib-2.0)
    pkgconfig(gobject-2.0)
    pkgconfig(gthread-2.0)
    pkgconfig(gtk+-3.0)
    plank(x86-32)
    vala-tools



Provides
--------
plank:
    application()
    application(plank.desktop)
    libplank.so.0
    plank
    plank(x86-32)

plank-devel:
    pkgconfig(plank)
    plank-devel
    plank-devel(x86-32)



Source checksums
----------------
https://launchpad.net/plank/1.0/0.5.0/+download/plank-0.5.0.tar.xz :
  CHECKSUM(SHA256) this package     : bf0e14dbdc9d30d57e55cc88644ef6e6c296be924bbfc1b345cba1e0c9be1804
  CHECKSUM(SHA256) upstream package : bf0e14dbdc9d30d57e55cc88644ef6e6c296be924bbfc1b345cba1e0c9be1804


Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13
Command line :/usr/bin/fedora-review -rvn plank-0.5.0-1.fc20.src.rpm
Buildroot used: fedora-rawhide-i386
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

--------------------------
1. valadoc is now ready for packaging, I will cleanup and then continue packaging it.  But I won't set it as a blocker anymore. Bug later ;)

2. /sbin/ldconfig is not needed for devel package.

3. %{_mandir}/man1/%{name}.1.gz --> %{_mandir}/man1/%{name}.1*

RPM will help you when it is uncompressed.

4. Group tag can be removed.(Not an issue)

--------------------------
Please paste the new spec/srpm with fix or explanation regarding the above issues.

Note that bamf will receive the update soon in rawhide, should have no affection to this package.
Comment 28 Christopher Meng 2014-02-15 06:35:09 EST
Please also remove the group tag in devel package since you've done that in the main package. 

PACKAGE APPROVED.
Comment 29 Wesley Hearn 2014-02-17 09:06:33 EST
New Package SCM Request
=======================
Package Name: plank
Short Description: A port of docky to vala
Owners: jknife
Branches: f20 rawhide
InitialCC:
Comment 30 Jon Ciesla 2014-02-17 09:56:50 EST
Git done (by process-git-requests).
Comment 31 Fedora Update System 2014-02-17 11:02:21 EST
plank-0.5.0-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/plank-0.5.0-4.fc20
Comment 32 Fedora Update System 2014-02-18 08:36:27 EST
plank-0.5.0-4.fc20 has been pushed to the Fedora 20 testing repository.
Comment 33 Fedora Update System 2014-02-26 08:59:37 EST
plank-0.5.0-4.fc20 has been pushed to the Fedora 20 stable repository.