[Bug 1398690] Review Request: perl-Module-Extract-Use - Pull out the modules a module explicitly uses

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

 



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



--- Comment #3 from Fabio Valentini <decathorpe@xxxxxxxxx> ---
(In reply to Paul Howarth from comment #2)
> Hi Fabio,
> 
> (In reply to Fabio Valentini from comment #1)
> > =====================
> > | !! NON-BINDING !! |
> > |  Package Review   |
> > =====================
> > 
> > I did this preliminary package review as part of the process of
> > becoming a fedora packager, so a "real" review is still needed.
> > 
> > IMO, besides the unneccessary BuildRequires, the review
> > looks simple enough. Regardless, a link to a successful koji
> > scratch build would have been nice.
> 
> OK, here's a scratch build:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=16647335
> 

Looks good!

> > Issues:
> > =======
> > - All build dependencies are listed in BuildRequires, except for any that
> >   are listed in the exceptions section of Packaging Guidelines.
> > 
> >   Note: These BR are not needed: coreutils make findutils
> >   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
> 
> It looks like this comment is a remnant from an old version of the packaging
> guidlines, which now say:
> 
> It is important that your package list all necessary build dependencies
> using the BuildRequires: tag. You may assume that enough of an environment
> exists for RPM to function, to build packages and execute basic shell
> scripts, but you should not assume any other packages are present as RPM
> dependencies and anything brought into the buildroot by the build system may
> change over time. 
> 
> (https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#BuildRequires_2)
> 
> So I think it is safest to include everything that is explicitly used during
> the package build.

Fair point, I did assume that fedora-review was up to snuff with the latest
packaging guidelines concerning this - obviously, I was wrong.

> > Reviewer's Comment: The first 32 lines of the .spec file are not nicely
> > formatted at all (indentation with 8-space-tabs instead of simple spaces, no
> > empty lines for better readability, etc.) - although it seems that the
> > .spec file has been adapted from another package or a Perl package template,
> > because many already existing / approved perl package .specs look that way.
> 
> This one is a matter of personal taste really and there are no guidelines
> about use of tabs, unless the use resulted in the spec not being legible to
> read. I think blocking on use of regular 8-space tabs (which I find helps
> line things up easily) would be stretching things a bit.

Of course. I just wanted to mention it as "comment only", because - depending
on the text editor configuration or environment - your .spec file might not
look as intended.

> Thanks for the feedback.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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