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 226161 - Merge Review: mrtg
Summary: Merge Review: mrtg
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:43 UTC by Nobody's working on this, feel free to take it
Modified: 2008-12-11 16:43 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-11 16:43:12 UTC
gwync: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:43:24 UTC
Fedora Merge Review: mrtg

http://cvs.fedora.redhat.com/viewcvs/devel/mrtg/
Initial Owner: mitr@redhat.com

Comment 1 Gwyn Ciesla 2008-09-16 16:21:58 UTC
rpmlint on SRPM:

mrtg.src:22: E: prereq-use vixie-cron
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

mrtg.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 44)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance.  Use either spaces or tabs for indentation, not both.

Fix.

mrtg.src: W: strange-permission filter-requires-mrtg.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

mrtg.src: W: strange-permission filter-provides-mrtg.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

What are these about?  If they're good this way, please document what they do in the spec.

rpmlint on RPMS:

mrtg.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/mrtg-2.16.2/contrib/whodo/GIFgraph/GIFgraph/samples/sample54.dat
mrtg.i386: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES

Tons of this all over the docs.  Fix.

mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualpri.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/hiperdsp.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualt1.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/cfgmaker_cisco/cfgmaker.cisco "/pkg/gnu/bin/perl"
This script uses an incorrect interpreter.

mrtg.i386: E: only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

mrtg.i386: W: dangerous-command-in-%trigger rm

Fix.

License tag should by GPLv2+.

Otherwise, full review looks good, no other blockers.

Comment 2 Gwyn Ciesla 2008-09-30 12:09:30 UTC
ccing current owner.

Comment 3 Gwyn Ciesla 2008-12-09 20:51:55 UTC
Ping?

Comment 4 Vitezslav Crhonek 2008-12-11 16:31:15 UTC
Hi,

(In reply to comment #1)
> rpmlint on SRPM:
> 
> mrtg.src:22: E: prereq-use vixie-cron
> The use of PreReq is deprecated. In the majority of cases, a plain Requires is
> enough and the right thing to do. Sometimes Requires(pre), Requires(post),
> Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

Fixed. Changed to plain Requires.

> 
> mrtg.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 44)
> The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
> annoyance.  Use either spaces or tabs for indentation, not both.
>

Fixed.

> 
> mrtg.src: W: strange-permission filter-requires-mrtg.sh 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.
> 
> mrtg.src: W: strange-permission filter-provides-mrtg.sh 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.
> 
> What are these about?  If they're good this way, please document what they do
> in the spec.

These files are used to filter perl provides/requires. I think it's not necessary to document them in spec, because it's often used and well known technique.
These files are also NOT listed to be included in final package.

For more info see:
https://fedoraproject.org/wiki/Packagin/Perl#External_filtering

> 
> rpmlint on RPMS:
> 
> mrtg.i386: W: wrong-file-end-of-line-encoding
> /usr/share/doc/mrtg-2.16.2/contrib/whodo/GIFgraph/GIFgraph/samples/sample54.dat
> mrtg.i386: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES
> 
> Tons of this all over the docs.  Fix.

I fixed only this one:
mrtg.x86_64: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES

Because CHANGES is real documentation file. The rest is in /usr/share/doc/mrtg-2.16.2/contrib/* - just ideas for mrtg users over different operating systems. I feel it's right to make it available but untouched.

> 
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualpri.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/hiperdsp.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualt1.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/cfgmaker_cisco/cfgmaker.cisco
> "/pkg/gnu/bin/perl"
> This script uses an incorrect interpreter.

I have same reason as above to don't touch these files (often examples from other operating systems).

> 
> mrtg.i386: E: only-non-binary-in-usr-lib
> There are only non binary files in /usr/lib so they should be in /usr/share.

There are only perl modules in /usr/lib{64}/* and this it's AFAIK standard place for them and mrtg will search them here.

> 
> mrtg.i386: W: dangerous-command-in-%trigger rm
> 
> Fix.

This trigger is used when very old mrtg (2.9.17) is uninstalled. This warning isn't real problem... But it was added in Feb 2003, so I think it's time to remove it completely.

> 
> License tag should by GPLv2+.

License changed to GPLv2+.

> 
> Otherwise, full review looks good, no other blockers.

Changes are commited to the devel branch. Let me know if you have any comments or anything else, otherwise feel free to close this review.

Comment 5 Gwyn Ciesla 2008-12-11 16:43:12 UTC
Very good, thanks!

APPROVED


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