Bug 582106 - Review Request: lifeograph - Private digital diary.
Summary: Review Request: lifeograph - Private digital diary.
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: 596433
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-04-14 06:33 UTC by Gowrishankar Rajaiyan
Modified: 2010-06-30 15:08 UTC (History)
5 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2010-06-11 16:29:33 UTC
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Gowrishankar Rajaiyan 2010-04-14 06:33:40 UTC
Spec URL: http://gowrishankar.fedorapeople.org/spec/lifeograph.spec
SRPM URL: http://gowrishankar.fedorapeople.org/srpms/lifeograph-0.6.2-1.fc12.src.rpm
Description: 

Upstream URL: https://launchpad.net/lifeograph
Lifeograph is an off-line and private journal and note taking application.


Note: Debug-info package that gets created in empty.

Comment 1 Mamoru TASAKA 2010-05-01 18:36:20 UTC
Some notes:

* Using %name / %version macro in SourceURL
  - Using %name / %version (especially %version) in SourceURL
    is generally useful:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* Debuginfo rpm
  - Creating debuginfo rpm is a must and please fix this
  ! Note
    build.log shows
------------------------------------------------------------------------------
    77  g++ ../obj/*.o -O2 -s `pkg-config gtkmm-2.4 gconfmm-2.6 gtkspell-2.0 --cflags --libs` -lgcrypt -lgpg-error -o ../bin/lifeograph
------------------------------------------------------------------------------
    and "-s" option strips the binary.
    Passing 'LINKFLAGS=""' to make resolves this issue (for
    this package).

* Timestamps
  - This package installs many image files which are not modified during
    rpmbuild and keeping timestamps on those files is recommended.
    For this package, using
------------------------------------------------------------------------------
make debug install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
------------------------------------------------------------------------------
    will keep timestamps on those files

    ! Note
      - For this package I don't think src/makefile need modifying,
        you can change the valiable used in src/makefile by passing
        option to make like:
------------------------------------------------------------------------------
make %{?_smp_mflags} OPTFLAGS="%{optflags}" LINKFLAGS=""
------------------------------------------------------------------------------
        and for install see above

* $RPM_BUILD_ROOT vs %buildroot
  https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
  - Please choose one and don't use both.

* desktop-file-install / desktop-file-validate
  - Once a desktop file is installed using desktop-file-install, there is
    no need to use desktop-file-validate for the installed desktop file
    (the installed desktop file is already verified).

* Directory ownership issue
  https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
  https://fedoraproject.org/wiki/Packaging/UnownedDirectories

  - The following directories are not owned by any packages:
-----------------------------------------------------------------------------
%{_datadir}/pixmaps/lifeograph/
%{_datadir}/pixmaps/lifeograph/backgrounds/
-----------------------------------------------------------------------------

Comment 2 Mamoru TASAKA 2010-05-12 15:35:02 UTC
ping?

Comment 3 Gowrishankar Rajaiyan 2010-05-13 09:51:40 UTC
Hi Mamoru,

I appreciate and thank you for reviewing the spec. Shall submit the new spec file asap.

Thank you once again.

Comment 4 Mamoru TASAKA 2010-05-21 16:40:05 UTC
ping again?

Comment 5 Gowrishankar Rajaiyan 2010-05-24 12:49:18 UTC
Hi Mamoru,

I am very sorry for the delay.
New spec and srpm has been uploaded to the following location:

Spec URL: http://gowrishankar.fedorapeople.org/spec/lifeograph.spec
SRPM URL: http://gowrishankar.fedorapeople.org/srpms/lifeograph-0.6.2-1.fc12.src.rpm

Comment 6 Mamoru TASAKA 2010-05-26 18:24:56 UTC
(Currently build for dist-f14 (rawhide) fails due to
 bug 596433)

Comment 7 Mamoru TASAKA 2010-05-26 18:45:37 UTC
From next time please change the release number every time
you modify your spec file (when version number doesn't change)
to avoid confusion, even during under review process.


Now for new -1:

! Comments
  - It is better that you remove comments which are no longer
    needed for readability.

! %clean section
  - If you want to import this package into F-12, %clean section
    is still needed.

* Duplicate files entry:
---------------------------------------------------------
   145  warning: File listed twice: /usr/share/pixmaps/lifeograph
   146  warning: File listed twice: /usr/share/pixmaps/lifeograph.png
   147  warning: File listed twice: /usr/share/pixmaps/lifeograph/backgrounds
   148  warning: File listed twice: /usr/share/pixmaps/lifeograph/backgrounds/bg.png
   149  warning: File listed twice: /usr/share/pixmaps/lifeograph/backgrounds/bg.png
---------------------------------------------------------
  - Note that the %files entry
---------------------------------------------------------
%files
%{_datadir}/pixmaps/lifeograph
---------------------------------------------------------
    contains the directory %_datadir/pixmaps/lifeograph and all 
    files/directories/etc under this directory.
    Also this package should not own the directory %{_datadir}/pixmaps itself.

Comment 8 Mamoru TASAKA 2010-05-26 18:51:16 UTC
Now:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

I see your another review request for webmit (bug 586327), however
currently the spec file is too complicated and needs refresh. So
I would expect another review request from you or your pre-review
of other person's review request.

Comment 9 Gowrishankar Rajaiyan 2010-05-27 15:04:38 UTC
Hi Mamoru,

New spec file and srpm have been uploaded.

spec: http://gowrishankar.fedorapeople.org/spec/lifeograph.spec
srpm: http://gowrishankar.fedorapeople.org/srpms/lifeograph-0.6.2-2.fc12.src.rpm


I shall definitely pre-review of other person's review request or file a new review request at the earliest.

Thank you once again.

Comment 10 Mamoru TASAKA 2010-05-27 17:28:01 UTC
(Now this package builds also on rawhide)

For -2:

* %clean section
  - '[ "${RPM_BUILD_ROOT}" != "/" ] &&' part is not needed.

* %changelog
  - It is recommended that you put one line between each %changelog
    entry like:
------------------------------------------------------------------
* Thu May 27 2010 Gowrishankar Rajaiyan <gsr@redhat.com> - 0.6.2-2
- Update the spec file as per Mamoru's review in comment 7 of BZ-582106.

* Mon May 24 2010 Gowrishankar Rajaiyan <gsr@redhat.com> - 0.6.2-1
- Update the spec file as per Mamoru's review in comment 1 of BZ-582106. 
  Forgot to update the version number.

* Fri Apr 09 2010 Gowrishankar Rajaiyan <gsr@redhat.com> - 0.6.2-1
- initial spec
------------------------------------------------------------------

(In reply to comment #9)
> I shall definitely pre-review of other person's review request or file a new
> review request at the earliest.
> 
> Thank you once again.  

Thanks.

Comment 11 Gowrishankar Rajaiyan 2010-05-28 08:29:03 UTC
Hi Mamoru,

New spec file and srpm have been uploaded.

spec: http://gowrishankar.fedorapeople.org/spec/lifeograph.spec
srpm: http://gowrishankar.fedorapeople.org/srpms/lifeograph-0.6.2-3.fc12.src.rpm

Thank you.

Comment 12 Gowrishankar Rajaiyan 2010-05-29 06:48:12 UTC
Hi Mamoru,


As promised here is my first pre-review:
https://bugzilla.redhat.com/show_bug.cgi?id=592864#c2

Comment 13 Mamoru TASAKA 2010-05-29 17:12:00 UTC
Well, I checked the above review request and actually the above
review has a lot of things to fix. I posted some comments on
the bug, please check those.

Anyway I will sponsor you.

-------------------------------------------------------------
    This package (lifeograph) is APPROVED by mtasaka
-------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 12/13, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.

Comment 14 Mamoru TASAKA 2010-06-06 16:58:34 UTC
ping?

Comment 15 Gowrishankar Rajaiyan 2010-06-09 16:02:07 UTC
New Package CVS Request
=======================
Package Name: lifeograph
Short Description: Private digital diary
Owners: gowrishankar sundaram
Branches: F-13 F-12
InitialCC:

Comment 16 Kevin Fenzi 2010-06-11 04:49:34 UTC
CVS done (by process-cvs-requests.py).

Comment 17 Fedora Update System 2010-06-11 14:17:47 UTC
lifeograph-0.6.2-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/lifeograph-0.6.2-3.fc13

Comment 18 Mamoru TASAKA 2010-06-11 16:29:33 UTC
Closing.

Comment 19 Fedora Update System 2010-06-30 15:08:27 UTC
lifeograph-0.6.2-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.


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