[Bug 993636] Review Request: perl-Text-BarGraph - ASCII bargraph generator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Joshua Small <technion@xxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |technion@xxxxxxxxxxx



--- Comment #1 from Joshua Small <technion@xxxxxxxxxxx> ---
Hi,

This is an informal review as I cannot sponsor.

URLS: It would be appreciated if you listed a direct download URL. At present,
the URL given embeds the .spec file within a website. Attempting to wget the
"direct download" link requires it be escaped and then provides a file named
"perl-Text-BarGraph.spec?format=raw".

BuildRoot:      %{_tmppath}/%{name}-%{version}-build
The above line is obsolete and should not be used.

%define cpan_name Text-BarGraph
Source0:        %{cpan_name}-%{version}.tar.gz
I believe this macro is quite redundant and could be addressed as a single
line.

# Text-BarGraph-1.1.tar.gz was downloaded from
http://www.cpan.org/authors/id/K/
KB/KBAUCOM/Text-BarGraph-1.1.tar.gz, LICENSE file ammended to archive

Am I correct in the understanding that you downloaded from the above URL, then
altered the tarball with a LICENSE file? This means there is no programmatic
method of downloading the upstream source and creating the one you used.

A more correct method would be to define Source1: as containing the LICENSE
file, then Source0: could be a proper URL.

I believe this would be more in line with
http://fedoraproject.org/wiki/Packaging:SourceURL which states:
One of the design goals of rpm is to cleanly separate upstream source from
vendor modifications. For the Fedora packager, this means that sources used to
build a package should be the vanilla sources available from upstream. To help
reviewers and QA scripts verify this, the packager needs to indicate where a
reviewer can find the source that was used to make the rpm. 

However, this leads me to the concern that you have listed license as:
License:        GPL+ or Artistic
Everything in the actual source suggests it is using Artistic Clarified.

find . -type f -print0 | xargs -0 chmod 644
I'm unsure what value the above line adds to the .spec file.

The package fails to actually build in koji which should be a clear blocker:
http://kojipkgs.fedoraproject.org//work/tasks/7717/5787717/build.log

What I'm seeing is that "make pure_install" doesn't pay attention to the build
root.
DESTDIR=%{buildroot} might fix it depending on the Makefile. You may be able to
remove this line altogether as you appear to have used the "install" command to
copy appropriate files.




I'm unable to review further due to the inability to build.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=uSTJS79NXE&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]