[Bug 1187869] Review Request: kdocker - Dock any application in the system tray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1187869



--- Comment #5 from Raphael Groner <projects.rg@xxxxxxxx> ---
 Rex Dieter 2015-02-24 16:00:29 CET

Offhand I don't see many MUST blockers, mostly SHOULD/polish type stuff:

1.  SHOULD apply patch instead of using sed statement:
# upstream forgot to bump version
sed -i 's,\(APP_VERSION = "\).*";,\1%{version}";,' src/constants.cpp

that's safer for when/if future versions land to tell if this is still needed
or not

2.  in %build, SHOULD use %{qmake_qt4} macro (instead of %{_qt4_qmake}), as
this one sets build flags, etc... instead of just calling plain 'qmake'
this macro is relatively new, and only available in fedora (not epel yet).  If
you want to keep things merged with epel too, you could use a construct like:
%{?qmake_qt4:%{qmake_qt4}}%{!?qmake_qt4:%{_qt4_qmake}}
(ie, use %qmake_qt4 if defined, else use %_qt4_qmake)

3. MUST not own /etc/bash_completion.d/
As a matter of fact, best practice is to install bash_completion items in what
is pointed at by output of:
pkg-config --variable=completionsdir bash-completion
/usr/share/bash-completion/completions
will want to add
BuildRequires: bash-completion
to ensure that pkg-config call works properly during the build.

4. MUST use system-copy of qtsingleapplication, not bundled copy under
solutions/qtsingleapplication

5. SHOULD (only mild suggestion) put app icon under
/usr/share/icons/hicolor/128x128/apps/
instead of legacy/deprecated
/usr/share/pixmaps
(and add relevant icon scriptlets)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]