[Bug 1267036] Review Request: perl-Mojolicious-Plugin-AssetPack - Compress and convert css, less, sass, javascript and coffeescript files

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

 



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



--- Comment #2 from Emmanuel Seyman <emmanuel@xxxxxxxxx> ---
(In reply to Petr Šabata from comment #1)
>
> * Correct the lettercase in your description.
>  `CSS', `Less', `Sass', `JavaScript' and `CoffeeScript'.

Done.

> * Missing a builddep called in the spec file: make

Added.

> * Since you're using the NO_PACKLIST feature, require at least EU::MM 6.76.

Indeed. Added.


> * Missing builddeps needed for the %check phase:
>  - perl(Fcntl), lib/Mojolicious/Plugin/AssetPack/Asset.pm:5
>  - perl(IO::File), lib/Mojolicious/Plugin/AssetPack/Asset.pm:6
>  - perl(Mojo::EventEmitter),
> lib/Mojolicious/Plugin/AssetPack/Preprocessors.pm:70
>  - perl(Mojo::UserAgent), lib/Mojolicious/Plugin/AssetPack.pm:26
>  - perl(Mojolicious::Plugin), lib/Mojolicious/Plugin/AssetPack.pm:3
>  - perl(POSIX), lib/Mojolicious/Plugin/AssetPack/Preprocessor.pm:19

I've added all of these.

> * Carp is not used anywhere.  You may (and should) drop it.

Removed.

> * Missing an undetected runtime dependency:
>  - perl(Mojo::UserAgent), lib/Mojolicious/Plugin/AssetPack.pm:26

Added.

> * I see many tests are skipped due to missing preprocessors.  Makes me
>   wonder whether this module actually works for these.
>   You may want to require some or all of them (if available), both at
>   build and runtime.

I was unwilling to add these as runtime requirements because users would end up
having to install all of these even if they just want to pack one type of
files. Maybe this is a case for soft requirements.

Adding them as BuildRequires has no impact on users so I've added those.

> * Consider running the optional POD tests (see t/00-basic.t).

Done.

> * Consider packaging the examples directory as %doc.

Done.

Spec URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-AssetPack/perl-Mojolicious-Plugin-AssetPack.spec
SRPM URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-AssetPack/perl-Mojolicious-Plugin-AssetPack-0.68-2.fc22.src.rpm

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review




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