Bug 475755 - Review Request: devtodo - Manage a hierarchical, prioritised list of outstanding tasks, jobs, or just reminders.
Summary: Review Request: devtodo - Manage a hierarchical, prioritised list of outstand...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrick Monnerat
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 477152 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-10 10:29 UTC by Bernie Innocenti
Modified: 2009-05-11 14:38 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-05-11 14:38:31 UTC
Type: ---
Embargoed:
patrick: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Bernie Innocenti 2008-12-10 10:29:05 UTC
Spec URL: http://codewiz.org/pub/fedora/pkgs/devtodo.spec
SRPM URL: http://codewiz.org/pub/fedora/pkgs/devtodo-0.1.20-1.src.rpm
Description:
Todo is a program to display and manage a hierarchical, prioritised list of
outstanding work, or just reminders.

The program itself is assisted by a few shell scripts that override default
builtins. Specifically, cd, pushd and popd are overridden so that when using
one of these commands to enter a directory, the todo will display any
outstanding items in that directory.

For much more complete information please refer to the man page (devtodo(1)).

Comment 1 Patrick Monnerat 2008-12-18 15:44:45 UTC
Some remarks:

rpmlint devtodo.spec:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint devtodo-0.1.20-1.src.rpm:
devtodo.src: W: summary-ended-with-dot Manage a hierarchical, prioritised list of outstanding tasks, jobs, or just reminders.
devtodo.src: E: summary-too-long Manage a hierarchical, prioritised list of outstanding tasks, jobs, or just reminders.
devtodo.src: W: no-version-in-last-changelog
1 packages and 0 specfiles checked; 1 errors, 2 warnings.

--> Please shorten the summary and suppressed the final "."
--> You also have to version changelog comments.
--> Release should end with "%{?dist}"

rpmlint devtodo-0.1.20-1.i386.rpm
devtodo.i386: W: non-conffile-in-etc /etc/profile.d/scripts.tcsh
devtodo.i386: E: standard-dir-owned-by-package /etc/profile.d
devtodo.i386: W: non-conffile-in-etc /etc/profile.d/scripts.sh
devtodo.i386: W: summary-ended-with-dot Manage a hierarchical, prioritised list of outstanding tasks, jobs, or just reminders.
devtodo.i386: E: summary-too-long Manage a hierarchical, prioritised list of outstanding tasks, jobs, or just reminders.
devtodo.i386: W: no-version-in-last-changelog
devtodo.i386: W: conffile-without-noreplace-flag /etc/todorc
1 packages and 0 specfiles checked; 2 errors, 5 warnings.

--> /etc/profile.d is owned by package setup, so you should not mention "%dir %{_sysconfdir}/profile.d" in you package files list.
--> Please use "%config(noreplace) for file %{_sysconfdir}/todorc, or explain why you don't use it.
--> The name "scripts.sh" and "scripts.tcsh" are too "impersonal" in system-wide directory %{_sysconfdir}/profile.d: i.e. change them to "devtodo.sh" and "devtodo.csh".

Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1006248

--> %doc:
	_ INSTALL should not be packaged.
	_ %{_mandir}/... should be outside of %doc
	_ %{_mandir}/... explicit names should be avoided since compression (thus name extension) is achieved by the rpmbuild macros. Use "%{_mandir}/man1/*".

License: Should probably be GPLv2, not GPLv2+: GPL v2 license is contained in
 the tarball, but no source file (except those generated by the autotools) menti
on it. I cannot find any note saying "GPL2 or later"...

Comment 2 Parag AN(पराग) 2008-12-19 12:31:58 UTC
*** Bug 477152 has been marked as a duplicate of this bug. ***

Comment 3 Bernie Innocenti 2008-12-19 13:11:29 UTC
Sorry for the low quality, it's an old spec file, and I also
ignored that running rpmlint on the spec file produces fewer
warnings.

All points have been addressed in this new release:

Spec URL: http://codewiz.org/pub/fedora/pkgs/devtodo.spec
SRPM URL: http://codewiz.org/pub/fedora/pkgs/devtodo-0.1.20-2.src.rpm

Comment 4 Patrick Monnerat 2008-12-19 16:37:41 UTC
SRPM link above is lame. You probably meant http://codewiz.org/pub/fedora/pkgs/devtodo-0.1.20-2.fc10.src.rpm

rpmlint devtodo.spec:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint devtodo-0.1.20-2.fc10.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint devtodo-0.1.20-2.fc11.i386.rpm
devtodo.i386: W: non-conffile-in-etc /etc/profile.d/devtodo.sh
devtodo.i386: W: non-conffile-in-etc /etc/profile.d/devtodo.tcsh
devtodo.i386: W: incoherent-version-in-changelog 0.1.20-1 ['0.1.20-2.fc11', '0.1.20-2']
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

--> Consider %config(noreplace) for %{_sysconfdir}/profile.d scripts. This will allow customization and make rpmlint silent.
--> %changelog comment versioning still not OK !

%{buildroot}/etc/profile.d --> %{buildroot}%{_sysconfdir}/profile.d  (3 times)

Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1009624

_ Please comment your "buildfixes" patch.
_ Consider also using option -p when installing scripts to preserve their mtime.

OK  package meets naming and versioning guidelines.
OK  specfile is properly named, is cleanly written and uses macros consistently.
OK  source files match upstream:
	sha1:	003067a12139d712dbb3706069e0950a93ecaaf4  devtodo-0.1.20.tar.gz
	md5:	4a6241437cb56f237f850bcd2233c3c4  devtodo-0.1.20.tar.gz
OK  summary is OK.
OK  description is OK.
OK  dist tag is present.
OK  build root is OK.
OK  license field matches the actual license.
OK  license is open source-compatible.
OK  license text included in package.
OK  BuildRequires are proper.
OK  compiler flags are appropriate (unchanged).
OK  %clean is present.
--  The package does not meet the Packaging Guidelines (changelog version)
OK  package builds in Koji (rawhide).
OK  package installs properly.
OK  debuginfo package looks complete.
--  rpmlint is not silent (see above).
OK  final provides and requires are sane:
	devtodo = 0.1.20-2.fc11
	devtodo(x86-32) = 0.1.20-2.fc11
  =
        libc.so.6()
	libgcc_s.so.1()
	libm.so.6  
	libncurses.so.5  
	libreadline.so.5  
	libstdc++.so.6()
	libtinfo.so.5
OK  %check is not present; no test suite upstream. I was able to run programs from the command line and manage todo lists.
OK  no shared libraries are added to the regular linker search paths.
OK  owns the directories it creates.
OK  doesn't own any directories it shouldn't.
OK  no duplicates in %files.
OK  file permissions are appropriate.
OK  code, not content.
OK  documentation is small, so no -doc subpackage is necessary.
OK  %docs are not necessary for the proper functioning of the package.
OK  no headers.
OK  no pkgconfig files.
OK  no static libraries.
OK  no libtool .la files.

This is my first review !!! I hope I've seen everything and I'm not too severe :-)

Comment 5 Michal Nowak 2008-12-19 20:53:07 UTC
> License: Should probably be GPLv2, not GPLv2+: GPL v2 license is contained in
> the tarball, but no source file (except those generated by the autotools)
> mention it. I cannot find any note saying "GPL2 or later"...

I believe the License field should state "GPLv2", but from different reason :).

W.r.t the text from https://fedoraproject.org/wiki/Licensing#Good_Licenses

""
A GPL or LGPL licensed package that lacks any statement of what version that it's licensed under in the source code/program output/accompanying docs is technically licensed under *any* version of the GPL or LGPL, not just the version in whatever COPYING file they include. 
"""

The content of the COPYING file is not that important compared to what is stated in src files and docs. And frankly the only statement of exact license is in doc/devtodo.1.in

.\" todo is licensed under the GPL, version 2. A copy of the GPL should have been distributed with the source in the file COPYING



The pkg looks good to me, it's pretty similar to the one I did in now closed bug 477152.

Comment 6 Bernie Innocenti 2008-12-21 12:32:50 UTC
Thanks for the review.  Here's an updated spec with all the comments addressed:

http://www.codewiz.org/pub/fedora/specs/devtodo.spec
http://www.codewiz.org/pub/fedora/source/devtodo-0.1.20-3.fc10.src.rpm

Comment 7 Patrick Monnerat 2008-12-22 10:42:17 UTC
Koji scratch: http://koji.fedoraproject.org/koji/taskinfo?taskID=1015833

rpmlint silent

Review accepted

Comment 8 Patrick Monnerat 2009-01-23 10:53:45 UTC
Ping ?
Package has passed review successfully (comment #7).
Bernie, do you still plan to import this package in Fedora ?

Comment 9 Bernie Innocenti 2009-05-09 13:01:21 UTC
(In reply to comment #8)
> Ping ?
> Package has passed review successfully (comment #7).
> Bernie, do you still plan to import this package in Fedora ?  

Sorry, I hadn't noticed your comment in my bugmail, and eventually totally forgot about this package.

Requesting CVS now.

Comment 10 Kevin Fenzi 2009-05-09 20:45:56 UTC
Please add a cvs request template here so we know what you want. 
Reset the fedora-cvs flag when you are ready.

Comment 11 Bernie Innocenti 2009-05-10 13:30:18 UTC
New Package CVS Request
=======================
Package Name: devtodo
Short Description: Manage a hierarchical, prioritized list of outstanding tasks
Owners: bernie
Branches: F-11
InitialCC: bernie

Comment 12 Kevin Fenzi 2009-05-10 19:18:08 UTC
cvs done.

Comment 13 Bernie Innocenti 2009-05-11 14:38:31 UTC
Thanks!

Package imported, package built for devel.  Building for F-11 shortly.


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