https://bugzilla.redhat.com/show_bug.cgi?id=1062920 Mo Morsi <mmorsi@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mmorsi@xxxxxxxxxx Assignee|nobody@xxxxxxxxxxxxxxxxx |mmorsi@xxxxxxxxxx Flags| |fedora-review? --- Comment #2 from Mo Morsi <mmorsi@xxxxxxxxxx> --- Taking this one. Note Nitesh needs sponsorship. Overall package looks good, some nits - Please update this bug summary in include the package summary not the package description - Also for your other submissions the bug summary needs to include the full package name, eg 'rubygem-foo' not just 'foo' (this one is fine) - The %check section should appear after the %install section and before the %files section - Consider adding the following to the %install section > pushd %{buildroot}%{gem_instdir}/ > rm -rf Rakefile amq-protocol.gemspec benchmarks \ > .gitignore .travis.yml .rspec codegen profiling \ > generate.rb .gitmodules > popd and removing the following from the %files sections < %exclude %{gem_instdir}/Rakefile < %exclude %{gem_instdir}/amq-protocol.gemspec < %exclude %{gem_instdir}/benchmarks/* < %exclude %{gem_instdir}/.gitignore < %exclude %{gem_instdir}/.travis.yml < %exclude %{gem_instdir}/.rspec < %{gem_instdir}/codegen/* < %{gem_instdir}/profiling/* < %{gem_instdir}/generate.rb < %{gem_instdir}/.gitmodules - Currently the spec suite pulls in bundler and effin_utf8, both of which are unecessary deps (and in the later case not in fedora). Please add to the %check section right after the pushd but before the rspec > sed -i /effin_utf8/d Gemfile > sed -i /effin_utf8/d spec/spec_helper.rb > sed -i /bundler/d spec/spec_helper.rb - The %{gem_cache} file needs to be marked as %exclude - The LICENSE file should be in the main package (not the 'doc' subpackage and _not_ marked as %doc) - Please add spacing in your changelog entry (eg between your name and email and between the '-' and 'initial package'). See other specs for the correct format. (may seem small but there are often tools that parse this stuff) Other than that package looks good and was able to koji it up. Once you make these changes and post the updated spec and srpm (make sure to bump the release and add a new changelog entry) I'll do the official review using fedora-review. http://koji.fedoraproject.org/koji/taskinfo?taskID=6532053 -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review