Bug 1194628

Summary:	Review Request: perl-Module-Metadata-Changes - Manage a module's machine-readable Changes/CHANGES file
Product:	[Fedora] Fedora	Reporter:	Petr Pisar <ppisar>
Component:	Package Review	Assignee:	Petr Šabata <psabata>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	package-review, psabata
Target Milestone:	---	Flags:	psabata: fedora-review+ puiterwijk: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:	perl-Module-Metadata-Changes-2.05-1.fc23	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2015-02-24 12:31:13 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Petr Pisar 2015-02-20 12:19:17 UTC

Spec URL: https://ppisar.fedorapeople.org/perl-Module-Metadata-Changes/perl-Module-Metadata-Changes.spec
SRPM URL: https://ppisar.fedorapeople.org/perl-Module-Metadata-Changes/perl-Module-Metadata-Changes-2.05-1.fc23.src.rpm
Description:
Module::Metadata::Changes is a pure Perl module. It allows you to convert
old-style Changes/CHANGES files, and to read and write Changelog.ini files.

Fedora Account System Username: ppisar

Comment 1 Petr Šabata 2015-02-20 14:19:05 UTC

Note: All the patched files are read-only.  I assume the patching could fail, even though it currently doesn't.  Perhaps making them writable before patching would be a good idea.

Ad Patch1 -- according to Web Assets guidelines, Fedora webservers provide %{_webassetdir} in `/_sysassets', not `/.sysassets'.  Please, correct the patch.

Consider bumping the minimum required version of EU::MM to 6.76 and using NO_PACKLIST to simplify the SPEC.

The convert.t test doesn't actually convert anything.  Maybe you could enhance it a bit?  However, that's beyond the scope of this review.

Comment 2 Petr Pisar 2015-02-23 11:46:07 UTC

(In reply to Petr Šabata from comment #1)
> Note: All the patched files are read-only.  I assume the patching could
> fail, even though it currently doesn't.  Perhaps making them writable before
> patching would be a good idea.
> 
I added this write permissions to the spec file. Updated package is on the same address.

> Ad Patch1 -- according to Web Assets guidelines, Fedora webservers provide
> %{_webassetdir} in `/_sysassets', not `/.sysassets'.  Please, correct the
> patch.
> 
This was a bug in guidelines and has been already corrected <https://fedoraproject.org/w/index.php?title=Packaging%3AWeb_Assets&action=historysubmit&diff=404471&oldid=350293>. The name with the dot is correct one.

> Consider bumping the minimum required version of EU::MM to 6.76 and using
> NO_PACKLIST to simplify the SPEC.
> 
> The convert.t test doesn't actually convert anything.  Maybe you could
> enhance it a bit?  However, that's beyond the scope of this review.

I'm not much interested in this.

Comment 3 Petr Šabata 2015-02-23 12:22:46 UTC

(In reply to Petr Pisar from comment #2)
> (In reply to Petr Šabata from comment #1)
> > Note: All the patched files are read-only.  I assume the patching could
> > fail, even though it currently doesn't.  Perhaps making them writable before
> > patching would be a good idea.
> > 
> I added this write permissions to the spec file. Updated package is on the
> same address.

Ack.

> > Ad Patch1 -- according to Web Assets guidelines, Fedora webservers provide
> > %{_webassetdir} in `/_sysassets', not `/.sysassets'.  Please, correct the
> > patch.
> > 
> This was a bug in guidelines and has been already corrected
> <https://fedoraproject.org/w/index.
> php?title=Packaging%3AWeb_Assets&action=historysubmit&diff=404471&oldid=35029
> 3>. The name with the dot is correct one.

Fair enough.

> > Consider bumping the minimum required version of EU::MM to 6.76 and using
> > NO_PACKLIST to simplify the SPEC.
> > 
> > The convert.t test doesn't actually convert anything.  Maybe you could
> > enhance it a bit?  However, that's beyond the scope of this review.
> 
> I'm not much interested in this.

Okay then.

Approving.

Comment 4 Petr Pisar 2015-02-23 13:17:11 UTC

New Package SCM Request
=======================
Package Name: perl-Module-Metadata-Changes
Short Description: Manage a module's machine-readable Changes/CHANGES file
Upstream URL: http://search.cpan.org/dist/Module-Metadata-Changes/
Owners: ppisar jplesnik psabata
Branches: 
InitialCC: perl-sig

Comment 5 Patrick Uiterwijk 2015-02-24 10:04:49 UTC

Git done (by process-git-requests).

Comment 6 Petr Pisar 2015-02-24 12:31:13 UTC

Thank you for the review and the repository.