[Bug 207741] Review Request: colorsvn - subversion output colorizer

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: colorsvn - subversion output colorizer


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


faucamp at csir.co.za changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |faucamp at csir.co.za
OtherBugsDependingO|                            |163776
              nThis|                            |




------- Additional Comments From faucamp at csir.co.za  2006-10-11 05:49 EST -------
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

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.