[Bug 919265] Review Request: Bijiben - Note taking app

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

 



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

--- Comment #9 from Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> ---
Thanks for the quick update Pierre-Yves, this is looking much better already!


Remaining issues:
================

[!]: gtk-update-icon-cache is not invoked
     => See:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
     => You're missing the "/bin/touch --no-create ..." line in %post


[!]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
     => The license tag appears correct to me, but it is more complex than what
I've
        been used to, so I'd appreciate a confirmation from the legal team.

[!]: Package fails to build in mock because of missing:
         BuildRequires: desktop-file-utils

Note, you should try to use mock to build your packages, as it would have
caught this last problem for you. :)


Details:
========

--- 919265-bijiben.bak/srpm-unpacked/bijiben.spec    2013-03-13
12:26:50.367149275 +0800
+++ 919265-bijiben/srpm-unpacked/bijiben.spec    2013-03-15 11:34:06.642803119
+0800
@@ -1,14 +1,25 @@
-%define url_ver    %(echo %{version}|cut -d. -f1,2)

     => Heh, dropping the macro definition is another way of fixing the issue.
:)

 Name:        bijiben
 Version:    3.7.91
-Release:    1%{?dist}
+Release:    2%{?dist}
 Summary:    Simple Note Viewer
-License:    GPLv2+
+
+# Bijiben is GPLv3+ appart a few files "LGPLv2 or LGPLv3"

     => Pedantic typo fix: "apart"

+# And ligd is LGPLv2+
+License:    GPLv3+ and LGPLv3 and LGPLv2+

     => The license tag appears correct to me, but it is more complex than what
I've
        been used to, so I'd appreciate a confirmation from the legal team.

 Url:        http://www.gnome.org
-Source0:   
http://download.gnome.org/sources/%{name}/%{url_ver}/%{name}-%{version}.tar.xz
+Source0:   
http://ftp.gnome.org/pub/GNOME/sources/%{name}/3.7/%{name}-%{version}.tar.xz
+
+# Do not use GPLv2+, but GPLv3+
+# Done upstream
+Patch0: license-use-consistently-GPLv3.patch
+
+# Do not have libgd.so
+# Done upstream
+Patch1: static-libgd.patch

     => Just a random note, for the future: the guidelines normally recommend a
link to
        the upstream bug tracker or mailing-list where the patches have been
submitted
        or to the commits in the git web interface.
     => But given that you're upstream, and that a new release (3.7.92) will be
out soon,
        I certainly wouldn't block the review on this. Just for your
information. :)

 BuildRequires:    intltool
 BuildRequires:    itstool
+BuildRequires:    gettext
 BuildRequires:    glib2-devel >= 2.28.0
 BuildRequires:    gtk3-devel >= 3.5.19
 BuildRequires:    tracker-devel => 0.15.2
@@ -17,25 +28,40 @@
 BuildRequires:    libzeitgeist-devel >= 0.3.18
 BuildRequires:    webkitgtk3-devel >= 1.11.91
 BuildRequires:    libuuid-devel
+BuildRequires:    yelp-tools
+
+# libgd is not meant to be installed as a system-wide shared library.
+# It is just a way for GNOME applications to share widgets and other common
+# code on an ad-hoc basis.
+Provides: bundled(libgd)

     => Good marking of bundled library.

 %description
 Simple note editor which emphasis on visuals : quickly write
 notes, quickly find it back.

+
 %prep
 %setup -q
+%patch0 -p1
+%patch1 -p1
+
+autoreconf -i -f

 %build
 %configure \
     --disable-static
-make
+make %{?_smp_mflags}
+
     => Fixes parallel building.

 %install
-rm -rf %{buildroot}

     => Good riddance. :)

-make DESTDIR=%{buildroot} install
+make install DESTDIR=%{buildroot} INSTALL="/usr/bin/install -p"

     => Fixes preserving of timestamps.

-%clean
-rm -rf %{buildroot}

     => Good riddance. :)

+# Validates the .desktop
+desktop-file-validate %{buildroot}%{_datadir}/applications/%{name}.desktop

     => Good validation of the desktop file.
     => However, the package doesn't build any more, because you forgot to add
            BuildRequires: desktop-file-utils

+# Creates the file for all locales
+%find_lang %{name} --with-gnome

     => Good handling of locale files.

 %post
 /usr/bin/update-desktop-database &> /dev/null || :

     => You're missing the "/bin/touch --no-create ..." line in %post

@@ -44,48 +70,59 @@
 /usr/bin/update-desktop-database &> /dev/null || :
 if [ $1 -eq 0 ] ; then
     /usr/bin/glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null ||
:
+    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
+    /usr/bin/gtk-update-icon-cache -f %{_datadir}/icons/hicolor &>/dev/null ||
:
 fi

     => Good handling of installed icons.

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

     => Good handling of installed icons.

 %files
 %doc NEWS AUTHORS COPYING ChangeLog NEWS README
+%files -f %{name}.lang

     => Good handling of the locale files.

 %{_bindir}/%{name}
 %{_datadir}/applications/%{name}.desktop
 %{_datadir}/glib-2.0/schemas/org.gnome.bijiben.gschema.xml
+# co-own these directories
+%dir %{_datadir}/gnome-shell
+%dir %{_datadir}/gnome-shell/search-providers

     => Fixes directory ownership.

 %{_datadir}/gnome-shell/search-providers/bijiben-search-provider.ini
-%{_datadir}/help/C/%{name}

     => Good handling of the locale files.

 %{_datadir}/icons/hicolor/*/apps/%{name}.png
 %{_datadir}/icons/hicolor/*/apps/%{name}.svg
-%lang(ca) %{_datadir}/locale/ca/LC_MESSAGES/%{name}.mo
-%lang(cs) %{_datadir}/locale/cs/LC_MESSAGES/%{name}.mo
-%lang(es) %{_datadir}/locale/es/LC_MESSAGES/%{name}.mo
-%lang(fr) %{_datadir}/locale/fr/LC_MESSAGES/%{name}.mo
-%lang(gl) %{_datadir}/locale/gl/LC_MESSAGES/%{name}.mo
-%lang(it) %{_datadir}/locale/it/LC_MESSAGES/%{name}.mo
-%lang(pl) %{_datadir}/locale/pl/LC_MESSAGES/%{name}.mo
-%lang(pt) %{_datadir}/locale/pt_BR/LC_MESSAGES/%{name}.mo
-%lang(sl) %{_datadir}/locale/sl/LC_MESSAGES/%{name}.mo
-%lang(sr) %{_datadir}/locale/sr/LC_MESSAGES/%{name}.mo
-%lang(sr) %{_datadir}/locale/sr@latin/LC_MESSAGES/%{name}.mo
-%lang(zh) %{_datadir}/locale/zh_CN/LC_MESSAGES/%{name}.mo

     => Good handling of the locale files.

 %{_datadir}/%{name}
-%{_libdir}/%{name}/libgd.so

     => Great, you don't install it anymore, which fixes the unowned directory,
as
        well as the bogus Requires/Provides.

 %{_libexecdir}/%{name}-shell-search-provider
 %{_datadir}/dbus-1/services/org.gnome.Bijiben.SearchProvider.service
 %exclude %{_libdir}/%{name}/libgd.la

 %changelog
+* Wed Mar 13 2013 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.91-2
+- Patch to get rid of GPLv2+
+- Patch to use static link to libgd
+- Use find_lang
+- Remove define for url_ver
+- desktop-file-validate
+- update timestamp for icons
+- BuildRequire gettext
+- BuildRequire yelp-tools
+- Provides bundled libgd
+- Don't "clean"
+
 * Sun Mar 10 2013 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.91-1
 - Fix BuildRequires
 - Update desktop database
 - Add it
+
 * Sun Feb 17 2013 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.90-1
 - Bump release
+
 * Sat Feb 02 2013 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.5-1
 - Add cs. 
+
 * Mon Jan 14 2013 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.4-1
 - Add ca pt zh. Remove upstreamed patch.
+
 * Mon Dec 17 2012 Pierre-Yves Luyten <py@xxxxxxxxx> - 3.7.3-1
 - Initial package

-- 
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=1DPYkCwg2k&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]