Bug 1122941 - Review Request: rubygem-font-awesome-rails - An asset gemification of the font-awesome icon font library
Summary: Review Request: rubygem-font-awesome-rails - An asset gemification of the fon...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-24 12:56 UTC by Josef Stribny
Modified: 2016-01-04 05:53 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-11-18 09:00:52 UTC
mtasaka: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Josef Stribny 2014-07-24 12:56:06 UTC
Spec URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails.spec
SRPM URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails-4.1.0.0-1.fc22.src.rpm
Description: A font-awesome icon font library for the Rails asset pipeline.
Fedora Account System Username: jstribny

Comment 1 Mamoru TASAKA 2014-09-14 16:57:31 UTC
* rpmlint about E: script-without-shebang
  - There are some files with executable permission (including image
    files) which should not have. Please modify permission.

* %license
  - Now we prefer to use %license for licensing files.

* Unneeded Requires / Provides for rubygem related packages
  - As rubygem releated Requires /Provides dependencies are automatically
    generated on F-21 and above, writing these explicitly is
    no more needed.

! Latest version
  - (not a blocker) please consider to upgrade to the latest 4.2.0.0

Comment 2 Josef Stribny 2014-09-16 11:36:45 UTC
* rpmlint about E: script-without-shebang
  --> Fixed

* %license
  --> Fixed

* Unneeded Requires / Provides for rubygem related packages
  --> I know about that, but there is only one require and it's not on RubyGem, therefor it's not generated

* ! Latest version
  --> This is not possible if I want to reuse the files from already ready fontawesome-fonts package from fedora. It needs to have the same version. I can do updates only when fontawesome-fonts do them.


Spec URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails.spec
SRPM URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails-4.1.0.0-2.fc22.src.rpm

Comment 3 Mamoru TASAKA 2014-09-21 16:21:17 UTC
(In reply to Josef Stribny from comment #2)
> * Unneeded Requires / Provides for rubygem related packages
>   --> I know about that, but there is only one require and it's not on
> RubyGem, therefor it's not generated

- Well, (for sure I redownloaded and) actually your -1.fc22.src.rpm
  contains
--------------------------------
    13  Requires: ruby(release)
    14  Requires: ruby(rubygems)
    15  Requires: rubygem(railties) >= 3.2
    16  Requires: rubygem(railties) < 5.0
    27  Provides: rubygem(%{gem_name}) = %{version}
--------------------------------
  Perhaps you once created srpm, and the later you modified
  your spec file and uploaded them seperately...

  -2.fc22.src.rpm does not contain these, so it is okay.

> * ! Latest version
>   --> This is not possible if I want to reuse the files from already ready
> fontawesome-fonts package from fedora. It needs to have the same version. I
> can do updates only when fontawesome-fonts do them.

- Well, then version dependency should also be specified for
  "Requires: fontawesome-fonts".
- By the way, you can ping fontawesome-fonts to upgrade it, or
  you can offer co-maintainership for that.

Well some more comments for -2:
* rubygem-font-awesome-rails-doc.noarch: E: non-executable-script /usr/share/gems/gems/font-awesome-rails-4.1.0.0/Rakefile 0644L /usr/bin/env
  - First of all, I wonder if shebang is needed for Rakefile...
    Please either
    * remove shebang
    * or make this have executable permission
    * or remove Rakefile at all (usually Rakefile is alike "makefile" for
      autotools based packages, and usually we don't include makefile
      or so in binary rpm - also see below)

* Shipping test suite
  - Ruby guideline requests not to package test suite in binary rpm
    (like autotools based rpm):
    https://fedoraproject.org/wiki/Packaging:Ruby?rd=Packaging/Ruby#Running_test_suites
    See "Do not ship tests"

Comment 4 Josef Stribny 2014-09-22 06:10:50 UTC
> Well, then version dependency should also be specified for
  "Requires: fontawesome-fonts".

True, done.

> I wonder if shebang is needed for Rakefile...

Since it is part of -doc, no, I removed it.

> * Shipping test suite

I ship tests when upstream ships them.


Spec URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails.spec
SRPM URL: http://data-strzibny.rhcloud.com/obs/rubygem-font-awesome-rails-4.1.0.0-3.fc22.src.rpm

Comment 5 Mamoru TASAKA 2014-11-04 06:26:50 UTC
Sorry for LOOOOONG delay. Will review again.

Comment 6 Mamoru TASAKA 2014-11-04 06:27:58 UTC
Josef, would you update srpm to 4.2.0.0?

Comment 7 Josef Stribny 2014-11-04 08:11:15 UTC
Mamoru,

as I wrote earlier I have to stay intact with the fontawesome dep[0]. Should I drop it and bundle the fonts instead? I though we will now on at least try not to duplicate fonts and JS if we can.


[0] http://koji.fedoraproject.org/koji/packageinfo?packageID=17584

Comment 8 Mamoru TASAKA 2014-11-07 06:01:37 UTC
(In reply to Josef Stribny from comment #7)
> Mamoru,
> 
> as I wrote earlier I have to stay intact with the fontawesome dep[0].

Well, just I have forgotton that reason. Thanks for reminder.
Now reviewing...

Comment 9 Mamoru TASAKA 2014-11-07 07:34:00 UTC
Oops...

Error: Package: rubygem-font-awesome-rails-4.1.0.0-3.fc22.noarch (/rubygem-font-awesome-rails-4.1.0.0-3.fc22.noarch)
           Requires: fontawesome-fonts >= 4.1.0.0
           Available: fontawesome-fonts-4.0.3-1.fc21.noarch (fedora)
               fontawesome-fonts = 4.0.3-1.fc21
           Installing: fontawesome-fonts-4.1.0-1.fc21.noarch (local)
               fontawesome-fonts = 4.1.0-1.fc21
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest

Because:
$ rpmdev-vercmp "4.1.0.0" "4.1.0"
4.1.0.0 > 4.1.0

Requires: fontawesome-fonts >= 4.1.0.0 cannot be satisfied by
fontawesome-fonts-4.1.0-1.fc21 . Please modify this.

Comment 11 Mamoru TASAKA 2014-11-12 15:03:47 UTC
Okay.

--------------------------------------------------------
  This package (rubygem-font-awesome-rails) is
  APPROVED by mtasaka
--------------------------------------------------------

Comment 12 Josef Stribny 2014-11-13 10:10:15 UTC
Thanks.

New Package SCM Request
=======================
Package Name: rubygem-font-awesome-rails
Short Description: An asset gemification of the font-awesome icon font library
Upstream URL: https://github.com/bokmann/font-awesome-rails
Owners: jstribny
Branches: f21
InitialCC:

Comment 13 Gwyn Ciesla 2014-11-13 16:39:35 UTC
Git done (by process-git-requests).

Comment 14 Josef Stribny 2014-11-18 09:00:52 UTC
Thanks.


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