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