Bug 235802 - Review Request: remind - Sophisticated calendar and alarm program
Summary: Review Request: remind - Sophisticated calendar and alarm program
Keywords:
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 Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-04-10 06:31 UTC by Ray Van Dolson
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version: 03.00.24-3.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-25 23:25:36 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
tcallawa: fedora-cvs+


Attachments (Terms of Use)
Patch to not strip binaries (470 bytes, patch)
2007-04-13 22:52 UTC, Dan Young
no flags Details | Diff

Description Ray Van Dolson 2007-04-10 06:31:54 UTC
Spec URL: http://www.bludgeon.org/~rayvd/rpms/remind/remind.spec
SRPM URL: http://www.bludgeon.org/~rayvd/rpms/remind/remind-03.00.24-1.src.rpm
Description:

This is the remind program which is a very powerful scheduling program (different purpose than cron).  I've created this package with the intent of making it part of EPEL.  It is my first package however, so if it can or should be included in Fedora Extras or elsewhere, that would be fine as well.

Comment 1 Nigel Jones 2007-04-11 22:30:47 UTC
Hey, I'm willing to do a prereview (I'm awaiting a sponsor myself for my first
pac kages).

Items that need looking at (Pasted directly from
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines)

- MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.
:You should consider adding COPYRIGHT and README (basedir) to %doc

- MUST: A package must not contain any duplicate files in the %files listing.
:/usr/share/man/man1/tkremind.1.gz is in both %{name} and %{name}-gui, you might
want to reconsider the pattern your using to match man files.

SHOULD Items:

- SHOULD: The reviewer should test that the package builds in mock.
:I can't test with mock sorry

Hopefully someone that can do an official review will be able to approve the
package!

p.s. If this is your first package, then you'll need to look at
http://fedoraproject.org/wiki/PackageMaintainers/SponsorProcess

Comment 2 Ray Van Dolson 2007-04-11 22:58:26 UTC
Thanks for the feedback.  I have built this package under mock successfully and
will take a look at the other items you mentioned.

Hopefully someone will be willing to sponsor me. :-)  Have a few other packages
I'd like to create down the road.

One thing that wasn't super clear to me was how to tag this as an EPEL package.
 I guess it can be both, but there wasn't a way to choose a "New" component
under EPEL (or I missed it).

Comment 3 Dan Young 2007-04-13 22:52:46 UTC
Created attachment 152596 [details]
Patch to not strip binaries

I'm seeking a sponsor also, so consider this just pre-review comments.

The binaries are being stripped, leaving an empty debuginfo package. See
http://fedoraproject.org/wiki/Packaging/Debuginfo for details. My patch to
src/Makefile.in is attached.

Comment 4 Mamoru TASAKA 2007-04-22 11:10:40 UTC
Ray, would you update your spec/srpm?
I will check then check yours.

Comment 5 Ray Van Dolson 2007-04-25 23:47:14 UTC
Thank you everyone for the feedback.  I believe I've incorporated the suggested
changes.

SRPM: http://www.bludgeon.org/~rayvd/rpms/remind/remind-03.00.24-1.1.src.rpm
Spec: http://www.bludgeon.org/~rayvd/rpms/remind/remind.spec

TIA!

Comment 6 Mamoru TASAKA 2007-04-26 05:42:41 UTC
I will review this formally

-----------------------------------------------------
* MUST FIX or SHOULD FIX. If you have a reason you don't
  want to, please explain why.

? Question or suggestion. This is not a blocker
-----------------------------------------------------

For 03.00.24-1.1:
* Release number
  - Release number should be integer, except for some
    exceptions.

* Timestamps
  - man pages are installed without any modification
    during build stage and keeping timestamps on these files
    is recommended.
    For this package, it can be done by
------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -c -p"
------------------------------------------------------

* %check
  - There seems to be a test program.
------------------------------------------------------
[tasaka1@localhost remind-03.00.24]$ pwd
/home/tasaka1/rpmbuild/BUILD/remind-03.00.24
[tasaka1@localhost remind-03.00.24]$ cd src
[tasaka1@localhost src]$ make test
sh ../tests/test-rem
Remind:  Acceptance test PASSED
------------------------------------------------------
    Consider to add %check section.

* Documentation
  - IMO it is better to add the followings as documentation
------------------------------------------------------
ACKNOWLEDGEMENTS
------------------------------------------------------
  - The following documents are not needed
------------------------------------------------------
README  - Unlike normal software, this README explains how
          to install by themselves and this is not needed for
          rpm users.
------------------------------------------------------

? WWW scripts
  - docs/README.UNIX says:
------------------------------------------------------
The subdirectory "www" contains scripts for making a nice calendar
web server.  See the files README and Makefile in that directory.
------------------------------------------------------
   IMO it is better to install the files under www by proper
   procedure.

? Icons
  - For desktop file, it is recommended to install a icon which
    represents this packge together. Please contact upstream to
    install some icon.

Well, this package is almost okay. Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few 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 Extras package review requests which are waiting for someone to
review can be checked on:
https://bugzilla.redhat.com/bugzilla/buglist.cgi?cmdtype=runnamed&namedcmd=mtasaka-review-noone
NOTE: FE-NEW blockers are now not complete.

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

Comment 7 Ray Van Dolson 2007-05-04 00:29:05 UTC
Thanks for the review Mamoru.  I have implemented your MUST FIX entries above
and am communicating with the remind developers as to artwork that could be used
for a tkremind icon.

The www package I was not intending to include at all in the RPM.  If you think
I should, I suppose it should be a separate -www or -web subpackage.

And I will work on getting sponsored as well.  Thanks for your guidance!

Comment 8 Ray Van Dolson 2007-05-04 23:48:26 UTC
Folks, here is the web calendar application included with remind:

  http://www.bludgeon.org/remind/calendar.html

It requires that variables defining the server's latitude and longtitude be set
to work properly.  In my opinion, this application is somewhat silly and if it
should be included, maybe should be done so under the documentation.

I can package it up however and allow the user to define their lat/long via a
file in /etc/sysconfig and update the CGI stuff to make use of this.

Thoughts?

Comment 9 Mamoru TASAKA 2007-05-06 09:28:09 UTC
(In reply to comment #8)
> I can package it up however and allow the user to define their lat/long via a
> file in /etc/sysconfig and update the CGI stuff to make use of this.
> 
> Thoughts?

I leave it to how you think whether we should create www related
subpackages.

(In reply to comment #7)
> Thanks for the review Mamoru.  I have implemented your MUST FIX entries above
Please provide the URL of new spec/srpm.

> and am communicating with the remind developers as to 
> artwork that could be used
> for a tkremind icon.
Thank you (but this is not a blocker)
 
> And I will work on getting sponsored as well.  Thanks for your guidance!
Usually you can do a pre-review of other peoples' review request
to get sponsored (as commented in comment 6)



Comment 10 Ray Van Dolson 2007-05-07 16:38:06 UTC
Update:

New spec/SRPM location:

  http://www.bludgeon.org/~rayvd/rpms/remind/remind-03.00.24-2.src.rpm
  http://www.bludgeon.org/~rayvd/rpms/remind/remind.spec

I have decided to not build a -www subpackage at this point.  I did include the
www/ subdirectory as part of the documentation should individuals wish to make
use of it.

This version contains no official artwork as I have yet to receive feedback from
the mailing list.  I have also contacted the author directly.  I think this
could be added later if artwork is made available or is created.

Comment 11 Mamoru TASAKA 2007-05-13 15:07:49 UTC
Still I am waiting for you to do a pre-review of some other
packages.

Comment 12 Ray Van Dolson 2007-05-14 20:34:18 UTC
I have one in mind and will do the pre-review shortly!  Was gone over the
weekend unfortunately.

Thanks for following up.

Comment 13 Ray Van Dolson 2007-05-22 19:48:58 UTC
I have done a pre-review of a Fedora Extras candidate package here:

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

Comment 14 Ray Van Dolson 2007-05-22 19:50:00 UTC
Question for reviewer: I intended this package initially for EPEL; what is the
proper way to get the package included in both fedora-extras and EPEL?  Should I
create a separate bugzilla for EPEL inclusion?

Comment 15 Ray Van Dolson 2007-05-22 20:10:04 UTC
Disregard question.  Appears I should let the remind package out into the
fedora-extras "wild" for some time for testing, then resubmit to EPEL.

Comment 16 Jason Tibbitts 2007-05-22 20:12:46 UTC
Well, when this package is approved and you make the CVS request, you can
request EPEL branches but nothing requires that you branch for released Fedora
versions.

Comment 17 Mamoru TASAKA 2007-05-22 22:32:03 UTC
I once recheck this review request (remind) in this afternoon
(in Japan: UTC + 9h = EST + 13h)

Comment 18 Mamoru TASAKA 2007-05-23 05:54:42 UTC
* For remind 03.00.24-2:
  - Doc dependency rpmlint
----------------------------------------------------------------
[tasaka1@localhost remind-03.00.24]$ rpmlint remind
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/moon
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/hebdate
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/sunset
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/sunrise
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/calps
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/rem2html
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/hebps
W: remind spurious-executable-perm
/usr/share/doc/remind-03.00.24/www/sunrise.rem-DIST
W: remind spurious-executable-perm
/usr/share/doc/remind-03.00.24/www/cal_dispatch-DIST
W: remind spurious-executable-perm /usr/share/doc/remind-03.00.24/www/hebhtml
W: remind incoherent-version-in-changelog 3.00.24-2 03.00.24-2.fc7
W: remind doc-file-dependency /usr/share/doc/remind-03.00.24/www/rem2html
perl(Getopt::Long)
W: remind doc-file-dependency /usr/share/doc/remind-03.00.24/www/rem2html
/usr/bin/perl
--------------------------------------------------------------
   - Documentation files should not have executable permission.
     Please change the permission to 0644.

= For your pre-review
  - There is some perl specific way to describe spec file,
    however for now it is okay.

So, please fix doc dependency rpmlint for remind.

Comment 19 Ray Van Dolson 2007-05-23 23:43:05 UTC
That issue should now be fixed.

http://www.bludgeon.org/~rayvd/rpms/remind/remind-03.00.24-3.src.rpm
http://www.bludgeon.org/~rayvd/rpms/remind/remind.spec

I will read up on the perl specific requirements for spec files as well.

Thanks.

Comment 20 Mamoru TASAKA 2007-05-24 14:34:07 UTC
One more comments:
-------------------------------------------------
[tasaka1@localhost Reviewing]$ rpmlint
/var/lib/mock/LOCALRPMS/SRPMS/remind-03.00.24-3.fc7.src.rpm 
W: remind macro-in-%changelog check
W: remind macro-in-%changelog files
--------------------------------------------------
Please use %%check, %%files in %changelog description.

Other things are okay.
---------------------------------------------------
  This package (remind) is APPROVED by me
---------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account". 

At a stage, you will submit a request which tells sponsor
members that  you need a sponsor. After you do so, please let me know
on this bug for confirmation. Then I will sponsor you.

If you have some questions, please let me know.

Comment 21 Ray Van Dolson 2007-05-24 22:11:37 UTC
Thank you Mamoru.  I'd already applied for 'fedorabugs' in the account system
before.  I was unable to remove and re-add it to send out notification emails.

I did add 'cvsextras' however and noted that the notification for that went out.

My FAS account name is 'rayvd'

Thanks again for your assistance.

Comment 22 Mamoru TASAKA 2007-05-25 06:41:41 UTC
Okay, now I am sponsoring you.

Comment 23 Mamoru TASAKA 2007-05-28 13:48:16 UTC
ping?

Comment 24 Ray Van Dolson 2007-05-29 22:09:54 UTC
Sorry, we had a holiday weekend here.  I'll be taking the next step(s) in the 
next day or so.

Comment 25 Ray Van Dolson 2007-05-30 19:13:23 UTC
New Package CVS Request
=======================
Package Name: remind
Short Description: A sophisticated calendar and alarm program
Owners: rayvd
Branches: FC-6 EL-4 EL-5 F-7

Comment 26 Tom "spot" Callaway 2007-05-31 14:18:05 UTC
cvs done

Comment 27 Ray Van Dolson 2007-06-01 04:40:59 UTC
Mamoru, maybe you can help me with this issue:

  https://www.redhat.com/archives/fedora-devel-list/2007-May/msg02169.html

Awaiting response on the list as well.  Does my CVS request above look correct?

Comment 28 Mamoru TASAKA 2007-06-01 04:55:18 UTC
Well, I got this trouble on my packages twice!!

I don't have a right to fix this issue (and I don't know
how to do it), however what I did (for the second time) is
to write a error message on the review request ticket
and to set fedora-cvs to ? (see bug 237382)

Then the problem was fixed _silently_ after one day (perhaps
some CVS admin fixed it). You can check if remind is
added to koji buildsys
http://koji.fedoraproject.org/koji/packages
If remind is added, you should be able to rebuild this
on koji.


Comment 29 Mamoru TASAKA 2007-06-01 05:39:06 UTC
Well, now it seems to be fixed.

https://www.redhat.com/archives/fedora-devel-list/2007-June/msg00013.html
http://koji.fedoraproject.org/koji/packageinfo?packageID=4459

Please try again (you may have to wait for an hour).

Comment 30 Mamoru TASAKA 2007-06-05 16:28:33 UTC
I suppose you can close this bug (as NEXTRELEASE) now?

NOTE:
Please don't forget to request to "push to testing" on
bodhi updates system. It seems that you have not requested
to do so (according to the status of remind-03.00.24-3.fc7).

Comment 31 Fedora Update System 2007-06-25 23:25:30 UTC
remind-03.00.24-3.fc7 has been pushed to the Fedora 7 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.