Bug 1122941

Summary: Review Request: rubygem-font-awesome-rails - An asset gemification of the font-awesome icon font library
Product: [Fedora] Fedora Reporter: Josef Stribny <jstribny>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hhorak, package-review
Target Milestone: ---Flags: mtasaka: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-11-18 09:00:52 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 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.