Bug 747610 - Review Request: timeline - Displays and navigates events on a timeline
Summary: Review Request: timeline - Displays and navigates events on a timeline
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2011-10-20 13:54 UTC by Gwyn Ciesla
Modified: 2011-11-19 06:07 UTC (History)
2 users (show)

Fixed In Version: timeline-0.14.0-3.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2011-10-29 18:27:23 UTC
Type: ---
mtasaka: fedora-review+
gwync: fedora-cvs+

Attachments (Terms of Use)
spec file to install gettext .mo files (2.90 KB, text/plain)
2011-10-28 17:31 UTC, Mamoru TASAKA
no flags Details

Description Gwyn Ciesla 2011-10-20 13:54:30 UTC
Timeline is a cross-platform application for displaying and navigating 
events on a timeline.

SPEC: http://zanoni.jcomserv.net/fedora/timeline/timeline.spec
SRPM: http://zanoni.jcomserv.net/fedora/timeline/timeline-0.14.0-1.fc15.src.rpm

Comment 1 Mamoru TASAKA 2011-10-22 17:17:45 UTC
I will take this one. By the way I appreciate it if you have some time to my review request bug 744628.

Comment 2 Mamoru TASAKA 2011-10-22 19:15:53 UTC
Some notes

* License
  - README says that this software is GPLv3 only
  - icons/COPYING says that icons are under CC-BY-SA
  - So I think the License tag should be "GPLv3 and CC-BY-SA".

* Unneeded specification
  - If you don't intend to import this into EPEL, the following
    items are no longer needed and should be removed.
    - BuildRoot line
    - "rm -rf $RPM_BUILD_ROOT" at the top of %install
    - %clean section entirely

* BR
  - Is scons really needed?

* Requires
  - Usually explicitly writing "hicolor-icon-theme" is not required.
    (I don't oppose to leave it though)

* Macros
  - Please use macros when possible. For example /usr/share/man
    should be %{_mandir}.

" Scriptlets
  - Please follow:
    (gtk-update-icon-cache oon %post moved to %posttrans)

* Documents
  - "INSTALL" file is usually for people trying to build and install
    a package by themselves and not needed for people using rpm
    mechanism. Also please check the result of rpmlint.

* gettext .po file issue
  - Check release/deb/debian/patches/fix-paths.patch and try to change
    LOCALE_DIR in main/timelinelib/paths.py.

Comment 3 Gwyn Ciesla 2011-10-26 18:47:13 UTC
Fixed license tag.  I may build for EPEL.  Scons is for building translations, but I can't get find_lang to work.  Fixed scriptlet.

I've modified my patch for the po paths, but I thought we weren't supposed to drop things in /u/s/locale manually.

Comment 4 Mamoru TASAKA 2011-10-27 14:43:41 UTC
About locale files
 - Fedora review guideline requests that locale files must be handled
   with %find_lang. This eventually requests that locale files are
   under %_datadir/locale/..... . Also man 3 gettext says that the default
   path for locale files is there. So unless you have some reason,
   using %_datadir/locale is preferred.

  - I prefer to use "msgfmt -o foo.mo foo.po" manually if scons is used
    only to build gettext .mo files.

By the way, anyway would you upload (and post the URLs of) srpm 
and spec file you are currently working on?

Comment 5 Gwyn Ciesla 2011-10-27 15:50:26 UTC
Right, but find_lang finds nothing.

The current iteration:

SPEC: http://zanoni.jcomserv.net/fedora/timeline/timeline.spec
SRPM: http://zanoni.jcomserv.net/fedora/timeline/timeline-0.14.0-2.fc15.src.rpm

Comment 6 Mamoru TASAKA 2011-10-27 16:07:12 UTC
I will check this later (maybe tomorrow...)

Comment 7 Mamoru TASAKA 2011-10-28 17:31:16 UTC
Created attachment 530699 [details]
spec file to install gettext .mo files

* About gettext .mo files
  - So you have to just build .mo files with gettext command and install
    the created .mo files to appropriate locations. Please check the attached
    spec file (and also check the lines about 80-90 of SConstruct).

* Scriptlets
  - Fedora packaging guideline says "touch --no-create 
    %{_datadir}/icons/hicolor || : " is in %post and %postun, not in

Please check the attached spec file, modify scriptlets and add the needed fixes if you want, and then upload new spec / srpm again.

Comment 9 Mamoru TASAKA 2011-10-28 18:45:31 UTC
 * Please check scriptlets again.

   touch .... should be in %post and %postun, but gtk-update-icon-cache
   should be in %postun and %posttrans (both were in %post and %postun,
   however guideline changed on 2009-03).


 * Now I don't think BR: scons is needed.

Please fix above when importing this package into Fedora git.
    This pacakge (timeline) is APPROVED by mtasaka

Comment 10 Gwyn Ciesla 2011-10-28 18:52:38 UTC
You're right, I missed that, and I'll pull scons.

Thanks for the review!

New Package SCM Request
Package Name: timeline
Short Description: Displays and navigates events on a timeline
Owners: limb
Branches: f15 f16

Comment 11 Gwyn Ciesla 2011-10-28 18:56:57 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2011-10-28 19:39:14 UTC
timeline-0.14.0-3.fc15 has been submitted as an update for Fedora 15.

Comment 13 Fedora Update System 2011-10-28 19:39:22 UTC
timeline-0.14.0-3.fc16 has been submitted as an update for Fedora 16.

Comment 14 Fedora Update System 2011-10-28 21:30:55 UTC
timeline-0.14.0-3.fc16 has been pushed to the Fedora 16 testing repository.

Comment 15 Mamoru TASAKA 2011-10-29 18:27:23 UTC

Comment 16 Fedora Update System 2011-11-19 06:01:00 UTC
timeline-0.14.0-3.fc16 has been pushed to the Fedora 16 stable repository.

Comment 17 Fedora Update System 2011-11-19 06:07:27 UTC
timeline-0.14.0-3.fc15 has been pushed to the Fedora 15 stable repository.

Note You need to log in before you can comment on or make changes to this bug.