https://bugzilla.redhat.com/show_bug.cgi?id=989400 --- Comment #3 from Vít Ondruch <vondruch@xxxxxxxxxx> --- (In reply to Mamoru TASAKA from comment #2) > Hello, Thank you for initial comments! > > (In reply to Ken Dreyer from comment #1) > > Hi Mamoru, > > > > Thanks for submitting the package. Here's a couple questions. > > > > 1. In %prep, I haven't seen this "tmpunpackdir" convention in many other > > gems, and I don't see it referenced in the Ruby packaging guidelines. If > > this convention is necessary for mini_portile, could you please add a > > comment to the .spec file indicating why this is necessary? > > This is my convension, to explicitly clarify that unpacking gem file is > done under the directory created by %setup -q -c -T > (i.e. to make it sure that no files are left after %clean is done: > using %setup after unpacking gem is confusing, and I remember that > old rpm unpacked source on unexpected directory when %setup was not > yet called) That is good convention. We should similarly update the packaging guidelines. Although it would be worth of some macros, since the things gets a bit ugly. > > 2. There is a section in %prep where you run "chmod 0644" on all the .rb > > files. From what I can see by running "gem unpack" by hand on > > mini_portile-0.5.1.gem, the single .rb file is already 644. In other words, > > it looks to me as if this step is unnecessary. Can you remove it from your > > .spec file? > > For safety. I have seen many times that .rb's have executable permission. If it was me, I would correct only stuff rpmlint reports. Doing some actions without real need is superfluous and does .spec files less readable. But this is not blocker. > > 3. From what I read in the current Ruby packaging guidelines, "gem build" is > > supposed to happen in %build. In your package, it happens in %prep. If there > > is a reason for this deviation, can you please clarify it in a comment in > > the .spec? > > Because I clean up tmpunpack dir. Here I want to make it sure that no > garbage files are left. (note that gem build is to repackage gem, and not > building something actually) This is related to first point and it does not matter in which section the "build" is done. > > 4. I'm curious about this line: > > %doc %{gem_instdir}/[A-Z]* > > > > Why not just specify LICENSE.txt explicitly here? From my understanding of > > the Ruby packaging guidelines, the History.txt and README.rdoc files belong > > in the -doc subpackage. > > README.rdoc must be in main (this is why "README" exists). Also it is quite > common that "History" file is in main. I agree with Ken at this point, but we might disagree. This is not a blocker. > > 5. Regarding these lines: > > > > # Currently no useful > > %exclude %{gem_instdir}/examples/ > > > > The comment should read "Currently not useful". Also, can you please expand > > the comment to explain why it would not be useful to ship the example > > Rakefile in that directory? > > Rakefile is something like "Makefile" on autotool-based tarballs, which we > don't usually package into binary rpms (also calling "rake" is discouraged > in current guideline) This is example in its real sense, i.e. it pretty much corresponds with appropriate section of README.rdoc [1] and it is out of question that it should stay in -doc subpackage. [1] https://github.com/luislavena/mini_portile#how-can-i-combine-this-with-my-compilation-task -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=sAtRJIqML0&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review