Red Hat Bugzilla – Bug 850469
Review Request: rubygem-Ascii85 - Methods to encode/decode Adobe's binary-to-text encoding of the same name
Last modified: 2012-09-17 18:05:03 EDT
Spec URL: http://miroslav.suchy.cz/fedora/rubygem-Ascii85/rubygem-Ascii85.spec SRPM URL: http://miroslav.suchy.cz/fedora/rubygem-Ascii85/rubygem-Ascii85-1.0.1-3.fc17.src.rpm Description: Ascii85 is a simple gem that provides methods for encoding/decoding Adobe's binary-to-text encoding of the same name. Fedora Account System Username:msuchy koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4410283 $ rpmlint /tmp/tito-build/rubygem-Ascii85-1.0.1-3.fc17.src.rpm /tmp/tito-build/noarch/rubygem-Ascii85-1.0.1-3.fc17.noarch.rpm rubygem-Ascii85.src: W: invalid-url Source1: ascii85.1.pod.tgz 2 packages and 0 specfiles checked; 0 errors, 1 warnings.
I'll take it for a review.
* Man pages - Wouldn't be better to keep the .pod file in original form, i.e. no tarball? It is so small that the benefits (you can read it in your editor, it could be kept in git and you could diff it easily) would outweigh the negatives (yes, the tarball is few bytes smaller probably, but git compresses its repository anyway). Moreover, you could reference the %{SOURCE1} directly in pod2man command, instead of unpacking and then referring to it. * Do not delete %{gem_instdir}/bin - This is definitely bad idea: $ ascii85 /usr/bin/ascii85:23:in `load': cannot load such file -- /usr/share/gems/gems /Ascii85-1.0.1/bin/ascii85 (LoadError) from /usr/bin/ascii85:23:in `<main>' - The %{_bindir} executable is just stub generated by rubygems and the %{gem_instdir}/bin is the real executable. * BuildRequires: perl - Really? What is it good for? * Test suite - Cannot be executed - See your Koji link referenced above ;) - Adding "BuildRequires: %{_bindir}/rspec" could solve the issue - Not sure why are you using the -P option. Can't see any difference running the test suite with or without it. - I prefer to run the test suite in .%{gem_instdir} instead of %{buildroot}%{gem_instdir}. While not exclusively better, if there is some test suite debris, it will not get into the RPM at least. * Keep the documentation in its original location - I know [1], but unless standardized differently, I'd like to point it out. However not a blocker. * ruby -KU - There are two alternatives which might be better: RUBYOPT="-KU" gem build %{gem_name}.gemspec or LANG=en_US.utf8 gem build %{gem_name}.gemspec [1] http://lists.fedoraproject.org/pipermail/packaging/2012-August/008598.html
> man pages I use tito for packaging. And it include in src.rpm only files with extenstion from this list: '.tar.gz', '.tgz', '.tar.bz2', '.tar', '.zip', '.jar', '.gem', ".spec", ".patch" So I would either had to make patch from it or do it manualy. And since I'm lazy as hell I prefer easy maintenance. Therefore I created tar from those files. They have to be download from obsucure location so there is no way for rpmlint to check md5sum, so I think it make no harm. > BuildRequires: perl - What is it good for? For pod2man. > Test suite fixed > Keep the documentation in its original location I will rebel here, if it is not blocker. :) > -KU "LANG=en_US.utf8 gem" does not work for me "RUBYOPT="-KU"" works find, I used this. > Do not delete %{gem_instdir}/bin fixed Spec URL: http://miroslav.suchy.cz/fedora/rubygem-Ascii85/rubygem-Ascii85.spec SRPM URL: http://miroslav.suchy.cz/fedora/rubygem-Ascii85/rubygem-Ascii85-1.0.1-4.fc17.src.rpm
(In reply to comment #3) > > man pages > I use tito for packaging. And it include in src.rpm only files with > extenstion from this list: > '.tar.gz', '.tgz', '.tar.bz2', '.tar', '.zip', '.jar', '.gem', ".spec", > ".patch" > So I would either had to make patch from it or do it manualy. > And since I'm lazy as hell I prefer easy maintenance. Therefore I created > tar from those files. They have to be download from obsucure location so > there is no way for rpmlint to check md5sum, so I think it make no harm. May be time to update Tito to support .pod files? Tools you are using cannot be excuse to provide suboptimal .spec files. I already explained what harm it does. It would be the same as compressing .patch files etc. Moreover, I believe (although not 100% sure) that the tgz file will get always uploaded into the look-aside cache, which will be definitely less effective in handling of updates to this file. Actually, you should use the following line: Source1: http://rubyforge.org/tracker/download.php/7826/30313/29377/5301/ascii85.1.pod which would fix the one remaining rpmlint warning. > > BuildRequires: perl - What is it good for? > For pod2man. Negative. I tried to remove it and it still works. Please remove the BR: perl, unless you have evidence that it is needed on Fedora(RHEL) != 19 * Man generation flags - Could you please keep the flags suggested in the ticket where you took the manual pages? If you omit them, there is stated "Perl" all over the place, which is not true. * Separate license file - Could you pleas query upstream about separate license file? From the review, I have the feeling, that you are not using mock for testing your packages. May be you should consider to look into it. It is worth of the time spent to learn. Since these are more or less just cosmetic issues and the package looks good otherwise, I APPROVE the package. However, please consider to include fixes to my comments prior importing.
> Man generation flags fixed > Separate license file done: http://rubyforge.org/tracker/index.php?func=detail&aid=29629&group_id=7826&atid=30313 > > BuildRequires: perl This is because perl is in minimal Buildroot. Per discussion on irc flipped to BuildRequires: %{_bindir}/pod2man Thanks for the review.
New Package SCM Request ======================= Package Name: rubygem-Ascii85 Short Description: Methods to encode/decode Adobe's binary-to-text encoding of the same name Owners: msuchy Branches: F-18, F-17, F-16, EL-6 InitialCC:
Git done (by process-git-requests).
rubygem-Ascii85-1.0.1-5.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/rubygem-Ascii85-1.0.1-5.fc18
rubygem-Ascii85-1.0.1-5.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/rubygem-Ascii85-1.0.1-5.fc17
rubygem-Ascii85-1.0.1-5.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/rubygem-Ascii85-1.0.1-5.fc16
rubygem-Ascii85-1.0.1-7.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/rubygem-Ascii85-1.0.1-7.el6
rubygem-Ascii85-1.0.1-5.fc18 has been pushed to the Fedora 18 testing repository.
rubygem-Ascii85-1.0.1-5.fc17 has been pushed to the Fedora 17 stable repository.
rubygem-Ascii85-1.0.1-5.fc16 has been pushed to the Fedora 16 stable repository.
rubygem-Ascii85-1.0.1-7.el6 has been pushed to the Fedora EPEL 6 stable repository.
rubygem-Ascii85-1.0.1-5.fc18 has been pushed to the Fedora 18 stable repository.