[Bug 782560] Review Request: rubygem-ruby-shadow - *nix Shadow Password Module

[bugzilla at redhat.com]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=782560

Vít Ondruch <vondruch at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?                   |

--- Comment #35 from Vít Ondruch <vondruch at redhat.com> ---
(In reply to comment #34)
Slavek is on holidays until next week, but I see no problem if you will send
your comments here and we can push this review forward. The important think is
to put more eyes on the spec file then who does what.

(In reply to comment #33)
I'd love to see comment referencing the upstream commit with the license
change, as long as new version carrying this change is not released.

And several other comments:

* %{ruby_version}
  - I already expressed my concerns about %{ruby_version} above and they still
    apply.
  - Moreover, as of this discussion [1] on packaging list, we are likely going
    to drop the mandatory versioning of ruby(abi), which speaks more in favor
of
    hardcoding the ruby(abi)

* %install section
  - Wouldn't be better to install just files of interest instead of copying
    everything and then doing clean up?

* You should own all your directories and files
  - For F17+, you do not own the %{gem_extdir}/lib, so I would suggest you to
    change the line to:

    %{gem_extdir}

    which will own its whole content, including the .so file

* shadow.so should be owned by only one package
  - In ruby-shadow subpackage you own the shadow.so file, but that file is
    already owned by the main package, which is correct. Please drop the .so
    file ownership from ruby-shadow subpackage
  - Since the package will not own any file, it could be noarch at the end?
    This is weird :)

* Drop the %{gem_instdir}/*.so
  - This is build leftover and will be never used. Please drop the file.

* Move *gemspec into -doc subpackage
  - The %{gem_instdir}/*gemspec file is not needed by runtime, please move it
    into -doc subpackage

on one additional hint, the package, as of now will not work with {RHEL,EPEL}7
since the Ruby will be similar to F17+ in its layout. But this could be
neglected for this review :)



[1]
http://lists.fedoraproject.org/pipermail/packaging/2012-December/008798.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=5MzaIJHtV0&a=cc_unsubscribe