Bug 859994 - Review Request: rubygem-simple_form - Flexible and powerful components to create forms
Review Request: rubygem-simple_form - Flexible and powerful components to cre...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Vít Ondruch
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-24 10:58 EDT by Imre Farkas
Modified: 2016-09-20 01:02 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-12-20 11:22:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
vondruch: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Imre Farkas 2012-09-24 10:58:51 EDT
Spec URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form.spec
SRPM URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form-2.0.2-1.fc17.src.rpm
Description: SimpleForm aims to be as flexible as possible while helping you with powerful components to create your forms. The basic goal of SimpleForm is to not touch your way of defining the layout, letting you find the better design for your
eyes.
Fedora Account System Username: ifarkas
Comment 1 Vít Ondruch 2012-10-03 09:48:35 EDT
Hi Imre,

I am not sponsor, so I can't sponsor you, but here are some comments:

* Newer upstream version available
  - There is already 2.0.3 release available. Please consider update prior
    importing into Fedora

* The package should own all directories
  - In -doc subpackage, you should own %doc %{gem_docdir} instead of the ri
    and rdoc directories, since the %{gem_docdir} represents the root
    of the gem documentation (see [1, 2]).

* Use macros where possible
  - Please replace %{gem_instdir}/lib with %{gem_libdir}

* .yard-doc
  - There is no such folder in the gem. Please drop the .yard-doc line from the
    %files section, since the package builds just fine without it in mock or
    koji.

* Missing requires
  - There must be specified runtime dependency on RubyGems, since rubygems
    package owns the directory structure, where the gem is installed, i.e.:

    Requires: rubygems

  - There are missing other runtime dependencies. The gem specifies dependency
    on actionpack ~> 3.0 and activemodel ~> 3.0. Please fix the dependencies.


* Invalid characters
  - Please drop the sed line replacing the UTF charactes and instead use:

    LANG=en_US.utf8 gem build %{gem_name}.gemspec

* Test suite
  - There are only 4 failing tests due to missing country_select gem, so it is
    worth of enabling the test suite. Please place following lines into
    appropriate places of your .spec file to do so:

    BuildRequires: rubygem(mocha)
    BuildRequires: rubygem(minitest)
    BuildRequires: rubygem(railties)
    BuildRequires: rubygem(tzinfo)

    # Get rid of Bundler.
    sed -i "/require 'bundler\/setup'/d" test/test_helper.rb
    # These needs rubygem-country_select which is not packaged for Fedora
    # so commenting it out
    sed -i "/require 'country_select'/d" test/test_helper.rb
    sed -i '104,107 s|^|#|' test/form_builder/general_test.rb
    sed -i '5,17 s|^|#|' test/inputs/priority_input_test.rb
    sed -i '38,42 s|^|#|' test/inputs/priority_input_test.rb
    find ./test -name *_test.rb | xargs testrb -Itest

* rpmlint complains
  
  rubygem-simple_form-doc.noarch: E: backup-file-in-package /usr/share/gems
    /gems/simple_form-2.0.2/test/form_builder/general_test.rb.orig

  - Since this file is coming from the .gem file, you should probably query
    upstream for a fix

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
[2] https://fedoraproject.org/wiki/Packaging:Ruby#Macros
Comment 2 Imre Farkas 2012-10-08 08:57:23 EDT
Hi Vít,

Thanks for the review! Your comments was really valuable. I updated the spec file and the source rpm accordingly. You can find them at:
Spec URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form.spec
SRPM URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form-2.0.3-1.fc17.src.rpm
Comment 3 Vít Ondruch 2012-10-08 09:22:32 EDT
(In reply to comment #1)
> * .yard-doc
>   - There is no such folder in the gem. Please drop the .yard-doc line from
> the
>     %files section, since the package builds just fine without it in mock or
>     koji.

This item still applies. This exclude line should be removed.

* Macro forms of system executables SHOULD NOT be used [1]
  - Please use plain "rm" instead of %{__rm}

* Changelog
  - Although we are just in review process, it is good to update the
    %changelog entries appropriately, e.g. you should keep the 2.0.2-1 entry
    and add new 2.0.3-1 with description of changes.
  - For now, keep it as it is, but please remember for the future.


Otherwise, the package looks good and it can be approved, as soon as you get sponsored.

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
Comment 4 Imre Farkas 2012-10-08 09:36:09 EDT
Thanks, fixed  again, hopefully without introducing any new issue.

Spec URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form.spec
SRPM URL: https://raw.github.com/ifarkas/fedora_rpm_specs/master/rubygem-simple_form-2.0.3-1.fc17.src.rpm
Comment 5 Vít Ondruch 2012-10-08 10:10:50 EDT
BTW it builds in koji just fine: http://koji.fedoraproject.org/koji/taskinfo?taskID=4570875
Comment 6 Imre Farkas 2012-10-09 03:44:34 EDT
New Package SCM Request
=======================
Package Name: rubygem-simple_form
Short Description: Flexible and powerful components to create forms
Owners: ifarkas
Branches: f16 f17 f18
InitialCC:
Comment 7 Gwyn Ciesla 2012-10-09 07:01:34 EDT
Git done (by process-git-requests).
Comment 8 Fedora Update System 2012-10-09 08:24:07 EDT
rubygem-simple_form-2.0.3-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/rubygem-simple_form-2.0.3-1.fc18
Comment 9 Fedora Update System 2012-10-09 08:37:03 EDT
rubygem-simple_form-2.0.3-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/rubygem-simple_form-2.0.3-1.fc17
Comment 10 Fedora Update System 2012-10-09 08:48:37 EDT
rubygem-simple_form-2.0.3-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/rubygem-simple_form-2.0.3-1.fc16
Comment 11 Fedora Update System 2012-10-09 13:19:21 EDT
rubygem-simple_form-2.0.3-1.fc18 has been pushed to the Fedora 18 testing repository.
Comment 12 Pete Travis 2012-11-17 12:19:46 EST
I see this package made it to the repo, but the bug here is still blocking FE-NEEDSPONSOR. Imre, assuming you have a sponsor, can you clean up the tracker bug by removing the block?
Comment 13 Imre Farkas 2012-11-19 09:00:09 EST
(In reply to comment #12)
> I see this package made it to the repo, but the bug here is still blocking
> FE-NEEDSPONSOR. Imre, assuming you have a sponsor, can you clean up the
> tracker bug by removing the block?

Sure, it's no longer a blocker.
Comment 14 Fedora Update System 2012-12-20 11:23:00 EST
rubygem-simple_form-2.0.3-1.fc18 has been pushed to the Fedora 18 stable repository.

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