[Bug 430070] Review Request: evolution-rss - Evolution RSS Reader Plugin

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: evolution-rss - Evolution RSS Reader Plugin


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


mtasaka at ioa.s.u-tokyo.ac.jp changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|430978                      |
         AssignedTo|nobody at fedoraproject.org    |mtasaka at ioa.s.u-tokyo.ac.jp
             Status|NEW                         |ASSIGNED
               Flag|                            |fedora-review?




------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2008-02-17 10:56 EST -------
* License issue
----------------------------------------------------
src/evolution-import-rss.c	GPLv2 (strict)
 -> %_bindir/evolution-import-rss is GPLv2

The other parts are GPLv2+
----------------------------------------------------
  Conclusion:
  - Following 
    http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
    You should write as following in spec file:
----------------------------------------------------
..............
Group: Applications/Internet
License: GPLv2 and GPLv2+
..............
%files -f %{name}.lang
%defattr(-,root,root,-)
# Only the following binaries is under GPLv2. Other
# parts are under GPLv2+.
%{_bindir}/evolution-import-rss
%{_sysconfdir}/gconf/schemas/evolution-rss.schemas
..............
----------------------------------------------------


For 0.0.7-6:

* configure option
  - The following options are not needed for %configure.
----------------------------------------------------------------
--prefix="%{_prefix}" --sysconfdir="%{_sysconfdir}"  --libdir=%{_libdir} 
----------------------------------------------------------------
    You can check what %configure actually does by
    $ rpm --eval %configure .

* Timestamps
  - To keep timestamps on installed files, I recommend to use
----------------------------------------------------------------
make install DESTDIR="%{buildroot}" INSTALL="install -p"
----------------------------------------------------------------
    This method usually works for recent autotool-based Makefiles.

* ldconfig
  - Calling /sbin/ldconfig is not needed for %preun.

* rpmlint
  - You can use rpmlint (in rpmlint package) to detect some generic
    mistakes in your rpms.
    Currently:
----------------------------------------------------------------
evolution-rss.i386: W: non-conffile-in-etc /etc/gconf/schemas/evolution-rss.schemas
evolution-rss.i386: W: no-version-in-last-changelog
----------------------------------------------------------------
    - The first one can be ignored.
    - For second one:
      * Please also write the EVR in %changelog (please check
        http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs )
      * Also, I recommend to insert one blank line between each changelog
        entry like:
----------------------------------------------------------------
%changelog
* Sat Feb 16 2008 Lucian Langa <cooly at gnome.eu.org> - 0.0.7-6
- Drop gecko requirements till xulrunner is fixed

* Tue Feb 12 2008 Lucian Langa <lucilanga at gnome.org> - 0.0.7-5
- buildroot fixes
----------------------------------------------------------------
         This is useful when using Fedora CVS system.

* Some misc cleanup
  - For consistency, as you write
----------------------------------------------------------------
%pre
if [ "$1" -gt 1 ]; then
	export GCONF_CONFIG_SOURCE=`gconftool-2 --get-default-source`
	gconftool-2 --makefile-uninstall-rule \
		%{_sysconfdir}/gconf/schemas/%{name}.schemas >/dev/null || :
                                             ^^^^^^^^^^^^^^^
fi
----------------------------------------------------------------
     Please use %{name} in %files entry.


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.