Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 207741 - Review Request: colorsvn - subversion output colorizer
Summary: Review Request: colorsvn - subversion output colorizer
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2006-09-22 19:59 UTC by V. Klein
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-10-15 23:12:51 UTC


Attachments (Terms of Use)

Description V. Klein 2006-09-22 19:59:16 UTC
Spec URL: http://www.console-colors.de/downloads/colorsvn/colorsvn.spec
SRPM URL: http://www.console-colors.de/downloads/colorsvn/colorsvn-0.3.1-1.src.rpm
Description: colorsvn is the Subversion output colorizer. Colorsvn was extracted from kde-sdk and was extended with build process and configuration.

Comment 1 V. Klein 2006-09-22 20:02:51 UTC
This is my first package, so I am looking for a sponsor.

Comment 2 Francois Aucamp 2006-10-11 09:49:46 UTC
A quick review: (I'm not an official reviewer, but I'm interested in this :-) )

Review items:
* rpmlint output (run rpmlint with -i for more info):
E: colorsvn sourced-script-with-shebang /etc/profile.d/colorsvn-env.sh
E: colorsvn executable-sourced-script /etc/profile.d/colorsvn-env.sh 0755

* package meets naming guidelines
* specfile is properly named, uses macros fairly consistently (see NOTES below)
* package license is GPL
* license field in spec file matches actual license

* license file is NOT included in %doc (see NOTES, below)

* source md5sum matches upstream:
dca88aacc8e7437a9b39e2449dbe678f  colorsvn-0.3.1.tar.gz

* package does NOT build in mock (FC5/i386, FC5/x86_64):

<snip>
checking for svn... no
configure: error: svn is required
error: Bad exit status from /var/tmp/rpm-tmp.11087 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.11087 (%build)
</snip>

You need to add "subversion" as a BuildRequires, or modify the configure script

* BuildRequires is incomplete (see above comment)

* no %post and %postun sections (no need for them)
* package is not relocatable
* directory/file ownership correct
* no duplicates in %files, %defattr is proper (see NOTES below)
* proper %clean section
* uses $RPM_BUILD_ROOT consistently; macros used well
* no need for seperate -doc or -devel subpackages
* no .desktop file needed (not a GUI app)

* package does not meet Fedora packaging guidelines (due to errors mentioned above)

* code, not content
* no locales
* %docs are not necessary for the proper functioning of the package


NOTES:
* You don't have to explictly list each %doc entry; just seperate them with spaces
* Add COPYING to the %docs
* You can use %{buildroot} instead of $RPM_BUILD_ROOT for full macro consistency
(not actually required, just a suggestion)
* In %files, instead of %{_prefix}/bin/colorsvn, you can use %{_bindir}/colorsvn
(just a suggestion)

By the way, shouldn't this bug also block FE-NEW ? 
I'm adding the block, but please remove it if I am missing something... ;-)
Cheers

Comment 3 Francois Aucamp 2006-10-11 09:57:40 UTC
I forgot to add the following....
* rpm does NOT install properly (dependant on host setup):

Preparing...                ########################################### [100%]
        file /usr/bin/colorsvn from install of colorsvn-0.3.1-1 conflicts with
file from package kdesdk-3.5.4-1.0.fc5.kde

I know you mention the kde-sdk origins in your description, but iou might want
to add a "Conflicts" statement specifying this conflict in your spec file, for
people running KDE.


Comment 4 V. Klein 2006-10-14 11:00:30 UTC
Thanks for your review and suggestions!

I have made some changes on the spec file according to your suggestions. But I
really don't know what to do with the rpmlint error. I think it is not right to
add a shebang line to the script because all another scripts in this directory -
/etc/profile.d - don't have the shebang too. But if I am wrong, so correct me.

Cheers

Comment 5 Kevin Fenzi 2006-10-14 17:30:16 UTC
In reply to comment #3:

Note that extras packages should not conflict with core packages. 
If kdesdk provides /usr/bin/colorsvn (which it does), I fear this package will 
not be acceptable for extras.

What is the diffrence between this version and the one provided by kdesdk?



Comment 6 V. Klein 2006-10-15 08:50:00 UTC
(In reply to comment #5)
> In reply to comment #3:
> 

> What is the diffrence between this version and the one provided by kdesdk?
> 
Separated configuration file, minor changes in code. The intention was to
extract it from kdesdk because colorsvn can be used without the kde stuff.

> Note that extras packages should not conflict with core packages. 
> If kdesdk provides /usr/bin/colorsvn (which it does), I fear this package will 
> not be acceptable for extras.

You have right, both packages are in conflict. This request can be deleted,
don't know how to do it.

Comment 7 Kevin Fenzi 2006-10-15 23:12:51 UTC
Perhaps you could talk with the kde folks upstream and see if they would be 
willing to stop shipping it? If they do, please do resubmit this to extras, or 
reopen this review request. 




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