https://bugzilla.redhat.com/show_bug.cgi?id=989400 --- Comment #2 from Mamoru TASAKA <mtasaka@xxxxxxxxxxxxxxxxx> --- 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) > 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. > 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) > 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. > 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) -- 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=NZxfyhsmP0&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review