[Bug 957333] Review Request: quiterss - Qt-based RSS/Atom aggregator

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

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=957333

--- Comment #16 from Markus Mayer <LotharLutz@xxxxxx> ---
(In reply to comment #11)
> (In reply to comment #8)
> > Clarifications:
> > ===============
> > %build honors applicable compiler flags or justifies otherwise:
> > I don't see how the appropriate flags are passed to make.
> 
> qmake do this.
> Compare:
> 1. rpm --eval %optflags
> -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
> --param=ssp-buffer-size=4  -m32 -march=i686 -mtune=atom
> -fasynchronous-unwind-tables
> 2. qmake-qt4 && grep ^CXXFLAGS Makefile.Release
> CXXFLAGS      = -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
> -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom
> -fasynchronous-unwind-tables -O2 -Wall -W -D_REENTRANT $(DEFINES)
> 
Yes, you are right. A wasn't aware of this fact.

> > Package contains no bundled libraries without FPC exception:
> > qyursqltreeview is still used. As this is only one file, I don't insist of
> > packing it separately.
> 
> Fedora has no qyursqltreeview in repos:
> https://apps.fedoraproject.org/packages/s/qyursqltreeview
> So - this is simple statically linked 3rd party.
> 
As Kevin stated, packaging it would be the best solution. But I am fine with
bundling it

> > Package complies to the Packaging Guidelines:
> > See http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
> 
> See %postun, %posttrans and Requires: hicolor-icon-theme.
> 
The proposed scriptlets are:
%post
/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
if [ $1 -eq 0 ] ; then
    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :

I have only found 'BuildRequires: hicolor-icon-theme'.

> > License field in the package spec file matches the actual license:
> > LGPL (v2 or later): 3rdparty/qyursqltreeview/qyursqltreeview.cpp
> > GPL (v3 or later): src/plugins/clicktoflash.cpp
> 
> Yes, you are right.
> As LGPL is compatible with GPL
> (https://fedoraproject.org/wiki/Licensing:
> Main?rd=Licensing#GPL_Compatibility_Matrix) I will set License to GPLv3+.
> 
> > Package is named according to the Package Naming Guidelines:
> > The upstream project name is QuiteRSS!
> 
> Don't mix _application_ name with _package_ one:
> VLC > vlc
> MPlayer > mplayer
> Qt4 > qt
> Fedora > fedora-release
> 
Well, I have found QuiteRSS and quite-rss on their project site, but not
quiterss. As I don't see any reason not using one of them, I would stick to the
guideline.
> > Package use %makeinstall only when make install' ' DESTDIR=... doesn't work:
> > Does the other way really not work?
> 
> Really.
> Unlike other build systems (like cmake) qmake uses INSTALL_ROOT instead of
> DESTDIR.
>  
I see you don't use %makeinstall anymore. Your current way is perfectly right.
> > Uses parallel make:
> > Please pass %{?_smp_mflags} to make
> Oops, you are right #2.
> 
> So - I found 3 things to correct: license, smp_flags and qmake-qt4 (ok, ok!
> :-).

As stated above, I am not quite happy with the naming and the scriptlet. Except
these two, there are no other issues.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=VHoBaqBDj2&a=cc_unsubscribe
_______________________________________________
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]