Bug 207741

This is my first package, so I am looking for a sponsor.

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

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.

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

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?

(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.

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.