Bug 240942 - Review Request: calcurse - Text-based personal organizer
Review Request: calcurse - Text-based personal organizer
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-23 05:05 EDT by Nigel Jones
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-03 01:38:28 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jochen: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nigel Jones 2007-05-23 05:05:12 EDT
Spec URL: http://dev.nigelj.com/SRPMS/calcurse.spec
SRPM URL: http://dev.nigelj.com/SRPMS/calcurse-1.8-1.src.rpm
Description:
Calcurse is a text-based calendar and scheduling application. It helps 
keep track of events, appointments, and everyday tasks.

A configurable notification system reminds the user of upcoming 
deadlines, and the curses based interface can be customized to suit user 
needs.

rpmlint clean, noarch package.

N.B. I'm sure someone wants to tell me "but the URL is wrong" as the comment in the spec file says, rpmbuild throws a fit when there is a ? after the /, I'm going to try and find out if there is a fix or not.
Comment 1 Jochen Schmitt 2007-05-23 13:15:51 EDT
Good:
+ Naming seems ok.
+ Rpmlint is quite for source rpm.
+ Rpmlint is quite for binary rpm.
+ Rpmlint is quite for debuginfo rpm.
+ Package contains verbatin copy of the license text.
+ Local install and uninstall works fine.
+ Startup of the application works without a crash.
+ Mock build works fine.

Bad:
- Source0 is not a full qualified URL.

Comment 2 Nigel Jones 2007-05-23 17:23:09 EDT
Hi Jochen!

(In reply to comment #1)
> Bad:
> - Source0 is not a full qualified URL.
Per my first message there is a reason behind this.

(In reply to comment #0)
> N.B. I'm sure someone wants to tell me "but the URL is wrong" as the comment
in the spec file says, rpmbuild throws a fit when there is a ? after the /, I'm
going to try and find out if there is a fix or not.

#Real SourceURL is: http://culot.org/cgi-bin/get.cgi?%{name}-%{version}.tar.gz 
#but the ? annoys rpmbuild
Source0:        %{name}-%{version}.tar.gz

The issue that this fixes is that if you do not do that, rpmbuild will try and
extract get.cgi?calcurse-1.8.tar.gz, not a good result.
Comment 3 Mamoru TASAKA 2007-05-23 23:22:30 EDT
* Well, I checked the URL and for this case it is not a
  problem. This type of URL problem (i.e. the "correct" URL
  contains some formats which rpmbuild cannot treat correctly)
  became a matter of argument (but I don't remember when
  and at which mailing list for now...) and the conclusion was

  "That is okay. Write the correct URL as a comment and
   use the tarball (if needed, renamed) name to Source."

* By the way
-------------------------------------------------------
%configure
CFLAGS=$RPM_OPT_FLAGS make %{?_smp_mflags}
-------------------------------------------------------
Is "CFLAGS" in the second line needed? (%configure sets
CFLAGS before ./configure is called)
Comment 4 Nigel Jones 2007-05-24 01:00:51 EDT
(In reply to comment #3)
> * Well, I checked the URL and for this case it is not a
>   problem. This type of URL problem (i.e. the "correct" URL
>   contains some formats which rpmbuild cannot treat correctly)
>   became a matter of argument (but I don't remember when
>   and at which mailing list for now...) and the conclusion was
> 
>   "That is okay. Write the correct URL as a comment and
>    use the tarball (if needed, renamed) name to Source."
This was the same opinion I got from two maintainers on IRC
> 
> * By the way
> -------------------------------------------------------
> %configure
> CFLAGS=$RPM_OPT_FLAGS make %{?_smp_mflags}
> -------------------------------------------------------
> Is "CFLAGS" in the second line needed? (%configure sets
> CFLAGS before ./configure is called)
I think I found in this case it was needed, I'll double check before the package
is entered into CVS though, but I seem to recall that it was needed.
Comment 5 Ralf Corsepius 2007-05-24 04:19:10 EDT
(In reply to comment #4)
>> %configure
>> CFLAGS=$RPM_OPT_FLAGS make %{?_smp_mflags}
>> -------------------------------------------------------
>> Is "CFLAGS" in the second line needed? (%configure sets
>> CFLAGS before ./configure is called)
>I think I found in this case it was needed, I'll double check before the 
> package is entered into CVS though, but I seem to recall that it was needed.
It is not needed. %configure is enough

Remove it.
Comment 6 Nigel Jones 2007-05-30 01:08:32 EDT
Spec URL: http://dev.nigelj.com/SRPMS/calcurse.spec
SRPM URL: http://dev.nigelj.com/SRPMS/calcurse-1.8-2.src.rpm

I'm not actually sure where the CFLAGS make came from, but when I was test
building it was needed, but it builds okay still so here is an updated package.
Comment 7 Jochen Schmitt 2007-05-31 11:43:00 EDT
Good:
+ Sources matches with upstream.
+ Local build works fine.

For the not qualified Source0 expression, I can make an exception, becouse you
wrote, that this couse an issue.

It may be nice, if you can submit a bug report agains the rpm package.

*** APPROVED ***
Comment 8 Nigel Jones 2007-05-31 16:15:24 EDT
(In reply to comment #7)
> It may be nice, if you can submit a bug report agains the rpm package.
I agree, I will do that over the weekend

New Package CVS Request
=======================
Package Name: calcurse
Short Description: Text-based personal organizer
Owners: dev@nigelj.com
Branches: FC-6 FC-7 EL-4 EL-5
InitialCC: 

Comment 9 Tom "spot" Callaway 2007-06-01 16:46:49 EDT
cvs done.
Comment 10 Nigel Jones 2007-06-03 01:38:28 EDT
Built and done, thanks for the review!

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