[Bug 1455470] Review Request: rubygem-gettext-setup - Easier internationalization with fast_gettext

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

 



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

Vít Ondruch <vondruch@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+



--- Comment #3 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
* Group is not needed anymore
  - The Group tag should not be used for F26+ [1].

* Unnecessary build dependencies
  - If I am not mistaken, rubygem(rspec) will pull in all other rspec-*
    packages. Are these BR necessary?
  - I am not sure about webmock and rack-test. I removed the dependency,
    searched for some requires but found nothing. Are they needed?
  - I don't think you need simplecov for anything, since we don't really care
    about code coverage. I'd get rid of the dependency by st. like:

~~~
sed -i "/require 'simplecov'/ s/^/#/" spec/spec_helper.rb
sed -i "/SimpleCov.start/,/^end$/ s/^/#/" spec/spec_helper.rb
~~~

* Use executable instead of package dependencies.
  - This is jut minor nit and I'' leave it up to you, since it is more or less
    about style, but if the package requires some executables, such as 'git'
    or 'msgcmp' in this case, I prefer to require directly these executables,
    instead of the packages. E.g.:

~~~
BuildRequires: %{_bindir}/msgcmp
BuildRequires: %{_bindir}/git
~~~

Otherwise, the package looks good => APPROVE


[1]
https://fedoraproject.org/w/index.php?title=Packaging:Guidelines&diff=487247&oldid=487238

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux