Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=652396 --- Comment #2 from Marek Goldmann <mgoldman@xxxxxxxxxx> 2010-11-15 05:12:22 EST --- (In reply to comment #1) > * Licensing > - It seems that openhash/openhash.rb comes from hashery gem > ( the latest version of hashery gem is 1.3.0). Also the license > of lib/hashery/openhash.rb in hashery 1.3.0 is currently under > ASL 2.0. Yes, this is the current state of OpenHash. > - Would you tell me where openhash/openhash.rb (in boxgrinder-core > gem) came from? As you pointed out, it was used from hashery: https://github.com/rubyworks/hashery/blob/master/lib/hashery/openhash.rb I added small modification in line 88 and license header. > - Also would you show us if the license of openhash.rb is really > under MIT? It seems that they moved to Apache License at some point: https://github.com/rubyworks/hashery/commit/0934b591ee716235a17d4ee691257bed291e36fb I used the code before the license switch. > - And. bundling files in other projects is generally forbidden > and needs FPC's approval. Would you consider to package > rubygem-hashery seperately? Ouch. Yes, this sounds like a good idea but in my case still I would need to override the method_missing (which is ~50% of the code :)) code to meet my needs. > * BuildRoot line / %clean section > - BuildRoot line is no longer needed on Fedora and EPEL6 I added BuildRoot tag because rpmlint highlighted it as a warning. Removing. > - %clean section is no longer needed on Fedora 12+ and EPEL6. OK, good to know â this is my first package. > * Unneeded version specific dependency > - " >= 1.2" part on "R: rubygems" or so are all unneeded > ( Only that "= %{abi}" part on "R: ruby(abi)" is needed ) > https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires > ( Please see the last 2 sentences in this section ) OK, I'll remove unnecessary requires. > * Output file on %check > - If %check section produces some extra files, it is preferable > that %check section won't touch %buildroot tree to avoid > unneeded confusion on %files and will touch files under > %_builddir > ( i.e. In this case, it is preferable that gem is once installed > under %_builddir and then copy the whole tree to %buildroot > at %install, then execute %check under %_builddir) Right, I didn't know how to deal with that. Should it look like this? %install rm -rf %{buildroot} rm -rf %{_builddir}%{gemdir} mkdir -p %{_builddir}%{gemdir} gem install --local --install-dir %{_builddir}%{gemdir} \ --force --rdoc %{SOURCE0} mkdir -p %{buildroot}/%{gemdir} cp -r %{_builddir}%{gemdir}/* %{buildroot}/%{gemdir} %check pushd %{_builddir}/%{geminstdir} rake spec popd > * Ability to build > - Your srpm does not build on koji for dist-rawhide (at least). > At least, "BR: rubygem(rake) rubygem(open4)" is needed (for %check) > (and perhaps also rubygem(rspec)) OK, I'll take a look at this issue. Thanks for the first review, I'll fix my issues and get back to you. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- 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