Bug 738590

Taking this for a review.

* Update to the latest version
  - Please consider updating to the latest version of gem. It seems it should
    incorporate the failed tests fix.
* License is not clear
  - The license should be "Public Domain and MIT", however I rose the question
    upstream to clarify [1]
* Please keep the note [2] about the "bundled" ruby_gettext_extractor
  in the spec file
* Exclude the cached gem
  - The cached gem has no meaning in Fedora. I would suggest to use the 
    following line in your spec:
    
    %exclude %{gemdir}/cache/%{gemname}-%{version}.gem
* Keep Readme.md in the main package
  - The file contains the license information. It would be fine to keep it
    in the main package.
* Keep the VERSION file in main package
  - This file is required by runtime:

    irb(main):002:0> require 'rubygems'
    => true
    irb(main):003:0> require 'gettext_i18n_rails'
    Errno::ENOENT: No such file or directory - /usr/lib/ruby/gems/1.8/gems
            /gettext_i18n_rails-0.2.20/lib/../VERSION
	from /usr/lib/ruby/gems/1.8/gems/gettext_i18n_rails-0.2.20
            /lib/gettext_i18n_rails.rb:2:in `read'
	from /usr/lib/ruby/gems/1.8/gems/gettext_i18n_rails-0.2.20
            /lib/gettext_i18n_rails.rb:2
	from /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:59:in
            `gem_original_require'
	from /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:59:in
            `require'
	from (irb):3
	from /usr/lib64/ruby/1.8/x86_64-linux/rbconfig.rb:172
  - Note that it should not be marked by %doc macro in this case.

[1] https://github.com/grosser/gettext_i18n_rails/issues/37
[2] https://github.com/retoo/ruby_gettext_extractor/issues/4

(In reply to comment #2)
> * Update to the latest version
>   - Please consider updating to the latest version of gem. It seems it should
>     incorporate the failed tests fix.

Done.

> * License is not clear
>   - The license should be "Public Domain and MIT", however I rose the question
>     upstream to clarify [1]

You are right, modified to Public Domain and MIT.

> * Please keep the note [2] about the "bundled" ruby_gettext_extractor
>   in the spec file

Done.

> * Exclude the cached gem
>   - The cached gem has no meaning in Fedora. I would suggest to use the 
>     following line in your spec:
> 
>     %exclude %{gemdir}/cache/%{gemname}-%{version}.gem

Done.

> * Keep Readme.md in the main package
>   - The file contains the license information. It would be fine to keep it
>     in the main package.

Done.

> * Keep the VERSION file in main package
>   - This file is required by runtime:
> 
>     irb(main):002:0> require 'rubygems'
>     => true
>     irb(main):003:0> require 'gettext_i18n_rails'
>     Errno::ENOENT: No such file or directory - /usr/lib/ruby/gems/1.8/gems
>             /gettext_i18n_rails-0.2.20/lib/../VERSION
>  from /usr/lib/ruby/gems/1.8/gems/gettext_i18n_rails-0.2.20
>             /lib/gettext_i18n_rails.rb:2:in `read'
>  from /usr/lib/ruby/gems/1.8/gems/gettext_i18n_rails-0.2.20
>             /lib/gettext_i18n_rails.rb:2
>  from /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:59:in
>             `gem_original_require'
>  from /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:59:in
>             `require'
>  from (irb):3
>  from /usr/lib64/ruby/1.8/x86_64-linux/rbconfig.rb:172
>   - Note that it should not be marked by %doc macro in this case.
> 

Good catch! Done.

> [1] https://github.com/grosser/gettext_i18n_rails/issues/37
> [2] https://github.com/retoo/ruby_gettext_extractor/issues/4

SPEC: http://bkabrda.fedorapeople.org/gettext_i18n_rails/rubygem-gettext_i18n_rails.spec
SRPM: bkabrda.fedorapeople.org/gettext_i18n_rails/rubygem-gettext_i18n_rails-0.3.0-1.fc15.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3460621

(In reply to comment #3)

> > * License is not clear
> >   - The license should be "Public Domain and MIT", however I rose the question
> >     upstream to clarify [1]
> 
> You are right, modified to Public Domain and MIT.

It is not that simple. The original project contains MIT LICENSE file, while the source code itself says "the same license as Ruby". That is pretty confusing ...

Yes, you are right, I'm currently trying to solve this issue: https://github.com/retoo/ruby_gettext_extractor/issues/5.

One more nit. The "%{geminstdir}/init.rb" file seems to be Rails plugin initializer, which is not used in gem version of the library. If that is true, it should be moved into -doc subpackage or completely excluded. But this is not blocker.

Otherwise, the package looks good. So once the license is clarified, the package could be approved.

- The license has been clarified - the gettext_i18n_rails is now the official upstream and the problematic license note has been removed from ruby_gettext_extractor.rb, so everything is now under Public Domain.
- I moved the %{geminstdir}/init.rb to -doc subpackage as suggested, the package works fine without it.
- I also upgraded to the newest version and fixed dependencies accordingly.

SPEC: http://bkabrda.fedorapeople.org/pkgs/gettext_i18n_rails/rubygem-gettext_i18n_rails.spec
SRPM: http://bkabrda.fedorapeople.org/pkgs/gettext_i18n_rails/rubygem-gettext_i18n_rails-0.3.6-1.fc17.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3613621

rpmlint reports following warning:

rubygem-gettext_i18n_rails.src: W: strange-permission rubygem-gettext_i18n_rails.spec 0666L
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

* Please consider removing of "Requires: rubygem(gettext)", since it is rails
  application development dependency, not a runtime think.
* Similarly "Requires: rubygem(ruby_parser)" should be removed for the same
  reason. Unfortunately, this dependency is now enforced by upstream. I have
  already opened issue [1] because of this.

Nevertheless, since these are just minor nits, I APPROVE the package. Please fix them before commit if possible.


[1] https://github.com/grosser/gettext_i18n_rails/issues/49

New Package SCM Request
=======================
Package Name: rubygem-gettext_i18n_rails
Short Description: Simple FastGettext Rails integration
Owners: bkabrda
Branches: 
InitialCC:

Git done (by process-git-requests).

Package Change Request
======================
Package Name: rubygem-gettext_i18n_rails
New Branches: el6
Owners: tagoh
InitialCC:

talked to vondruch and got an approval to request a branch.

Git done (by process-git-requests).

The current package PoC, vondruch, is in http://fedoraproject.org/wiki/EPEL/ContributorStatusNo