Bug 1194628 - Review Request: perl-Module-Metadata-Changes - Manage a module's machine-readable Changes/CHANGES file
Summary: Review Request: perl-Module-Metadata-Changes - Manage a module's machine-read...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Šabata
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-02-20 12:19 UTC by Petr Pisar
Modified: 2015-02-24 12:31 UTC (History)
2 users (show)

Fixed In Version: perl-Module-Metadata-Changes-2.05-1.fc23
Clone Of:
Environment:
Last Closed: 2015-02-24 12:31:13 UTC
Type: ---
Embargoed:
psabata: fedora-review+
puiterwijk: fedora-cvs+


Attachments (Terms of Use)

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.


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