[Bug 1267037] New: Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

Bug ID: 1267037
           Summary: Review Request: perl-Mojolicious-Plugin-Bootstrap3 -
                    Mojolicious + http://getbootstrap.com/
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

Spec URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...
SRPM URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...
Description:
Mojolicious::Plugin::Bootstrap3 is used to include http://getbootstrap.com/
CSS and JavaScript files into your project.

Fedora Account System Username: eseyman

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

Emmanuel Seyman  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends On|                            |1267036

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1267036
[Bug 1267036] Review Request: perl-Mojolicious-Plugin-AssetPack - Compress
and convert css, less, sass, javascript and coffeescript files

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #1 from Petr Šabata  ---
* The package fails to build in mock -- see the next bullet.

* Define NO_PACKLIST or delete the .packlist files manually.
  You probably wanted to use the former but simply forgot to put there.

* Missing a builddep called in the spec file: make

* Missing a builddep used during the %check phase:
  - perl(Mojolicious::Plugin), lib/Mojolicious/Plugin/Bootstrap3.pm:233

* Mojolicious::Lite doesn't appear to be used anywhere; it's only
  mentioned in POD.

* According to META.*, the required version of Mojolicious::Plugin::AssetPack
  is 0.67, not 0.58.

* Mojolicious::Plugin::AssetPack won't be autodetected.
  Require it explicitly, including the correct version constraint.

* Even though it's not used directly, consider adding a runtime dependency
  on Mojolicious 6.00+.

* The minimum required version of Test::More is 0.88.  You could reflect
  that in your package as well.

* I understand it's the whole point of this package but this ships bundled
  fonts, JavaScript (jQuery and many others) and Sass content.
  That doesn't seem right.  The package should probably be patched to
  use the data provided by Fedora packages or acquire proper exceptions.

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

[e-mail]  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from [e-mail]  ---
Well, the bundling policy just got completely revised:

https://fedorahosted.org/fpc/ticket/575

even under the old rules, though, JavaScript bundling was explicitly allowed
and CSS bundling was allowed in practice. There are still specific requirements
for fonts, however:

https://fedoraproject.org/wiki/Packaging:Guidelines#Avoid_bundling_of_fonts_...

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1267037
Bug 1267037 depends on bug 1267036, which changed state.

Bug 1267036 Summary: Review Request: perl-Mojolicious-Plugin-AssetPack - Compress and convert CSS, Less, Sass, JavaScript and CoffeeScript files
https://bugzilla.redhat.com/show_bug.cgi?id=1267036

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |RAWHIDE

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #3 from [e-mail]  ---
Ping, anything happening here? This is the last dep I need to unblock openQA
for the official repos...

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #4 from Emmanuel Seyman  ---
Okay, I've updated the package to 3.3600 (the current version), fixed all the
issues that Petr pointed out and indicated that bootstrap and jquery are
bundled.

Adam, if you have any improvements to suggest on the bundling part, I'm all
ears.

Spec URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...
SRPM URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #5 from [e-mail]  ---
That looks fine to me, so far as I understand the new policy and the
discussions around it. 'Unbundling' this package might have interesting
consequences, though - things that use plugin-bootstrap but don't have write
access to the 'bundled' files would have to pin their dependencies on the
packages which actually provided the files, I think.

(the SUSE guys packaged openQA such that the assets are generated during
package build, and the app cannot write to the asset dir; the benefit is
reducing the amount of stuff the app needs write access to, but the drawback is
the package must be rebuilt any time bootstrap3 changes. I'm still deciding
whether to follow this for the Fedora package).

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #6 from Petr Šabata  ---
The SRPM link doesn't work, assuming it's actually
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...

All points besides bundling addressed.  Since I understand the current bundling
policy as `anything goes' and I'm not sure how this package is supposed to be
used, I'm willing to approve this and leave it to you.

However, I think the License tag should include the content license as well. 
So in this case it should be `Artistic 2.0 and MIT'.

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #7 from Emmanuel Seyman  ---
(In reply to Petr Šabata from comment #6)
> The SRPM link doesn't work, assuming it's actually
> http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/
> perl-Mojolicious-Plugin-Bootstrap3-3.3600-1.fc23.src.rpm

This is what happens when you do packaging work during a FOSDEM talk.
> However, I think the License tag should include the content license as well.
> So in this case it should be `Artistic 2.0 and MIT'.

Indeed.

Spec URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...
SRPM URL:
http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/...

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #8 from Petr Šabata  ---
(In reply to Emmanuel Seyman from comment #7)
> (In reply to Petr Šabata from comment #6)
> > The SRPM link doesn't work, assuming it's actually
> > http://people.parinux.org/~seyman/fedora/perl-Mojolicious-Plugin-Bootstrap3/
> > perl-Mojolicious-Plugin-Bootstrap3-3.3600-1.fc23.src.rpm
>
> This is what happens when you do packaging work during a FOSDEM talk.

:)
> > However, I think the License tag should include the content license as well.
> > So in this case it should be `Artistic 2.0 and MIT'.
>
> Indeed.

Okay, I'll approve this now.

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #9 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/perl-Mojolicious-Plugin-Bootst...

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

Emmanuel Seyman  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |RAWHIDE
        Last Closed|                            |2016-02-03 09:37:01

--- Comment #10 from Emmanuel Seyman  ---
Folks, this is in Rawhide and an update has been issued for F23.

Petr, thank you for the review. You went above and beyond the call of duty on
this one.

[Bug 1267037] Review Request: perl-Mojolicious-Plugin-Bootstrap3 - Mojolicious + http://getbootstrap.com/

[Red Hat Bugzilla]

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

--- Comment #11 from [e-mail]  ---
That's awesome! Now openqa is unblocked :) Thank you very much for all your
help with getting the deps packaged, Emmanuel!