[Bug 845799] Review Request: rubygem-hashr - Simple Hash extension to make working with nested hashes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=845799

--- Comment #4 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
(In reply to comment #3)
> Fixed:
> * Use %global instead of %define
> * Test suite
> * License
> * -doc subpackage
> * Please use the new RubyGems packaging macros

Thank you.

> ad /Please consider to use "%if 0%{?el6}%{?fc16}" macros instead of wordy
>     "%if 0%{?rhel} == 6 || 0%{?fedora} < 17"/
> I do not like the shorter notation. It is less readable and thefore
> maintainable (of course IMHO).
> And while 0%{?fedora} < 17 works for Fedora 15, shorter notation %{?fc16}. I
> know that F15 is not supported, but still.. I simply like the longer
> notation.

Ok, no problem.

> * Exclude gemcache
> per our IRC discussion I will keep it here. I like the sideefect of
> repackaging, where - in case package will get patch - the repackage gem
> contains those patch(es) and developers, which develop for Fedora on their
> macs, can use this gem, which contains these patch(es).

This is not show stopper, but something which should be re-discussed at
Ruby-SIG. Any input is welcome. 


* Do not own the whole %{gem_instdir}
  - I would suggest to be more specific and instead of %{gem_instdir} use
    %dir %{gem_instdir}. This is a bit more work, but it will allow you to spot
    some major changes when updating the gem.
  - This is not show stopper, but I'd like you to consider it.

* Test suite execution
  - You can simplify the test suite execution command:

    testrb -Ilib test/*_test.rb"

* Non-essential files should be moved to -doc subpackage
  - Please move the test suite into -doc subpackage, since it is not needed for
    runtime
  - Please move Gemfile*, Rakefile and README.md as well

* The -doc subpackage should require the main package
  - Requires: %{name} = %{version}-%{release}

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]