Bug 690580

Summary: Review Request: perl-Perl-Critic-More - Supplemental policies for Perl::Critic
Product: [Fedora] Fedora Reporter: Petr Pisar <ppisar>
Component: Package ReviewAssignee: Petr Šabata <psabata>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, paul, psabata
Target Milestone: ---Flags: psabata: fedora-review+
tibbs: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: perl-Perl-Critic-More-1.000-1.fc15 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-04-05 16:53:34 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 690569    

Description Petr Pisar 2011-03-24 17:26:29 UTC
Spec URL: http://ppisar.fedorapeople.org/perl-Perl-Critic-More/perl-Perl-Critic-More.spec
SRPM URL: http://ppisar.fedorapeople.org/perl-Perl-Critic-More/perl-Perl-Critic-More-1.000-1.fc16.src.rpm
Description:
This is a collection of Perl::Critic policies that are not included in the
Perl::Critic core for a variety of reasons.

Comment 1 Petr Šabata 2011-03-30 09:09:40 UTC
Package: perl-Perl-Critic-More
Version: 1.000
Release: 1.fc16
Sources: Perl-Critic-More-1.000.tar.gz
Patches: 
----------
Package successfully built locally.
Package successfully built in mock, fedora-rawhide-x86_64.
Package successfully built in mock, fedora-rawhide-i386.
Package successfully built in koji, dist-rawhide.

MUST items:
[  OK  ] Package does NOT include pre-built binaries or libraries
[  OK  ] Spec file is legible and written in American english
[  OK  ] Package successfully builds on at least one supported primary architecture
[  --  ] All ExcludeArch tags valid, referencing proper bug reports
[  OK  ] Package obeys FHS (with _libexecdir and /srv exceptions)
[  OK  ] No errors reported by rpmlint
[  OK  ] Changelog present and properly formatted
[  OK  ] Package does NOT include Packager, Vendor, Copyright or PreReq tags
[  OK  ] Source tags are working URLs and sources match upstream or justified otherwise
[  OK  ] Requires correct or justified otherwise
[  OK  ] BuildRequires correct or justified otherwise
[  OK  ] All file names are in proper UTF-8 encoding
[  OK  ] All plain text failes are in proper UTF-8 encoding
[  --  ] Large documentation files are located in doc subpackage
[  OK  ] All documentation prefixed with %doc
[  OK  ] Documentation is NOT executable
[  OK  ] No files in %doc are needed at run-time
[  --  ] Compiler flags honor Fedora defaults or are justified
[  --  ] Package generates useful debuginfo packages
[  --  ] Header files are placed in devel subpackage
[  --  ] Unversioned shared libraries are placed in devel subpackage
[  --  ] Pkgconfig files are placed in devel subpackage
[  --  ] Full-versioned Requires of the base package in subpackages
[  --  ] Package calls ldconfig in post and postun sections for all subpackages, if applicable
[  --  ] Static libraries are provided by static subpackage
[  OK  ] Package contains no static executables unless approved by FESCo
[  OK  ] Package does NOT bundle any system libraries
[  --  ] RPath not used for anything besides internal libraries
[  --  ] All config files are marked noreplace or justified otherwise
[  OK  ] No config files are located under /usr
[  --  ] Package contains a SystemV-compatible initscript
[  --  ] A GUI application installs a proper desktop file
[  --  ] All desktop files are installed by desktop-file-install or justified otherwise
[  OK  ] Package consistently uses macros
[  --  ] makeinstall macro is used only if make install DESTDIR=%{buildroot} does NOT work
[  --  ] Macros in Summary and description are expandable at build-time
[  --  ] globals used in place of defines
[  --  ] Locales handled correctly -- package requires gettext and uses find_lang, if applicable
[  --  ] Scriptlets are sane
[  OK  ] Package is not relocatable unless justified
[  OK  ] Package contains only acceptable code or content
[  OK  ] Package owns all the files and directories it creates, installs and/or uses unless those are already owned by another package
[  OK  ] files sections do NOT contain duplicate files except for licenses
[  OK  ] All files sections use defattr or justify otherwise
[  OK  ] Package does NOT cause any conflicts
[  OK  ] Package does NOT contain kernel modules
[  OK  ] Package does NOT bundle fonts or other general purpose data
[ NOTE ] Final Requires and Provides are sane

SHOULD items:
[  OK  ] The Summary does NOT end with a period
[  OK  ] Package does NOT include BuildRoot tag, clean section or buildroot removal in install section
[  OK  ] Package should preserve files timestamps
[  OK  ] Package does NOT explicitly BuildRequire bash, bzip2, coreutils, cpio, diffutils, fedora-release, findutils, gawk, gcc, gcc-c++, grep, gzip, info, make, patch, redhat-rpm-config, rpm-build, sed, shadow-utils, tar, unzip, util-linux-ng, which or xz
[  OK  ] Description does NOT consist of lines longer than 80 characters
[  --  ] Package uses parallel make
[  --  ] In case of a web application, package installs date into /usr/share instead of /var/www
[  --  ] All patches have a comment or an upstream bug link
[  --  ] Package installs manpages for all executables
[  OK  ] Package contains check section and all tests pass
[  ??  ] Package works as expected

NOTES:
------
Filter out perl(Readonly) underspecified dependency.

Comment 2 Petr Pisar 2011-04-04 13:40:25 UTC
Fixed package on the same URL.

Comment 3 Petr Šabata 2011-04-04 13:44:11 UTC
--- perl-Perl-Critic-More.spec 2011-03-24 18:23:20.000000000 +0100
+++ perl-Perl-Critic-More.spec.new 2011-04-04 15:37:30.000000000 +0200
@@ -26,6 +26,10 @@
 Requires:       perl(Perl::MinimumVersion) >= 0.14
 Requires:       perl(Readonly) >= 1.03
 
+# Remove underspecified dependencies
+%filter_from_requires /^perl(Readonly)\s*$/d
+%filter_setup
+
 %description
 This is a collection of Perl::Critic policies that are not included in the
 Perl::Critic core for a variety of reasons.

--
Approving.

Comment 4 Petr Pisar 2011-04-04 14:05:41 UTC
New Package SCM Request
=======================
Package Name: perl-Perl-Critic-More 
Short Description: Supplemental policies for Perl::Critic
Owners: ppisar mmaslano psabata
Branches: 
InitialCC: perl-sig

Comment 5 Jason Tibbitts 2011-04-05 15:25:00 UTC
Git done (by process-git-requests).

Comment 6 Petr Pisar 2011-04-05 16:53:34 UTC
Thank you for the review and the repository.

Comment 7 Paul Howarth 2011-04-30 08:48:02 UTC
Petr, could we have an F-15 branch for this? It's used for the author/developer tests of Perl::Critic itself (as you probably know).

Comment 8 Marcela Mašláňová 2011-05-02 05:55:29 UTC
Package Change Request
======================
Package Name: perl-Perl-Critic-More
New Branches: F-15
Owners: ppisar mmaslano psabata

Comment 9 Petr Pisar 2011-05-02 18:58:53 UTC
I'm not sure it's best idea to pull perl-Perl-Critic-More package into perl-Perl-Critic build-time dependencies, as it would create dependency cycle (perl-Perl-Critic-More BuildRequires perl-Perl-Critic for standard tests already). I think it makes more sense to run perl-Perl-Critic-More tests than author tests in perl-Perl-Critic.

Comment 10 Marcela Mašláňová 2011-05-03 07:44:14 UTC
(In reply to comment #7)
> Petr, could we have an F-15 branch for this? It's used for the author/developer
> tests of Perl::Critic itself (as you probably know).

I'm taking back my request for branch. What do you think about #9?

Comment 11 Paul Howarth 2011-05-03 08:16:30 UTC
As it stands I have the author tests disabled by default but runnable by using --with authortests. I was thinking of changing things so that the author tests were run by default but could be turned off using --without authortests but if there's a consensus against that I could leave things as they are. However, it would still be useful to have the package available for anyone wanting to run the author tests (which would include anyone doing development work on Perl::Critic).

Another point is that if the author tests were enabled by default, it would certainly not be the only circular dependency amongst the perl modules in Fedora, and circular dependencies themselves aren't so much of an issue in an incrementally built distribution, as opposed to one that needed bootstrapping.

Comment 12 Petr Pisar 2011-05-03 09:05:48 UTC
I have no problem in adding perl-Perl-Critic-More into F15, provided it does not imply upgrading other packages. (AFAIK it doesn't. Marcela, verify it, I have no Fedora available now).

The circular dependencies are problem when upgrading perl interpreter between incompatible versions. Once you push incompatible interpreter, you need to rebuild everything from scratch. I'm preparing a tool for scratch rebuild and I will be pushing to break these superfluous dependencies.

Regarding author tests: The one who develops Perl::Critic operates on sources usually and does not use Koji as part of development, thus I cannot see any interjection in these two build systems.

Comment 13 Marcela Mašláňová 2011-05-04 13:07:30 UTC
(In reply to comment #12)
> I have no problem in adding perl-Perl-Critic-More into F15, provided it does
> not imply upgrading other packages. (AFAIK it doesn't. Marcela, verify it, I
> have no Fedora available now).
> 
http://koji.fedoraproject.org/koji/taskinfo?taskID=3049683
At least build in F-15 passed.

Comment 14 Petr Pisar 2011-05-04 13:17:13 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > I have no problem in adding perl-Perl-Critic-More into F15, provided it does
> > not imply upgrading other packages. (AFAIK it doesn't. Marcela, verify it, I
> > have no Fedora available now).
> > 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3049683
> At least build in F-15 passed.

And now the run-time part: Are all Requires satisfied in F15?

Comment 15 Marcela Mašláňová 2011-05-04 13:47:56 UTC
Package Change Request
======================
Package Name: perl-Perl-Critic-More
New Branches: f15
Owners: ppisar mmaslano psabata

Comment 16 Jason Tibbitts 2011-05-05 18:20:54 UTC
Git done (by process-git-requests).

Comment 17 Petr Pisar 2011-05-05 19:44:18 UTC
Jason-san,

thank you for processing the request exactly as asked. Unfortunately, Marcela forgot to request InitialCC for perl-sig. Please add that one to f15 branch: 

Package Name: perl-Perl-Critic-More
Branches: f15
Owners: ppisar mmaslano psabata
InitialCC: perl-sig

Comment 18 Jason Tibbitts 2011-05-05 19:51:40 UTC
Should be good now.  As always with things I have to manually edit, please double check and let me know if there are any issues.

Comment 19 Fedora Update System 2011-05-06 11:30:29 UTC
perl-Perl-Critic-More-1.000-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/perl-Perl-Critic-More-1.000-1.fc15

Comment 20 Fedora Update System 2011-05-19 04:51:49 UTC
perl-Perl-Critic-More-1.000-1.fc15 has been pushed to the Fedora 15 stable repository.