[Bug 1186572] Review Request: rubygem-rmagick - Ruby binding to ImageMagick

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1186572



--- Comment #2 from Mamoru TASAKA <mtasaka@xxxxxxxxxxxxxxxxx> ---
Well,

(In reply to František Dvořák from comment #1)
> Interesting package with many tricks, and it looks good (exact launch of the
> tests, package rename, ...).
> 
> Issues to consider:
> 
> 1) %global should be preferred over %define
> [http://fedoraproject.org/wiki/Packaging:Guidelines#.
> 25global_preferred_over_.25define]
> 
> - %define is considered less "predictable" than %global (although I don't
> understand the details)

- For parametrized macro, %define is preferable (there seems some
  way to use %global even for this case, but it is messy)
  https://www.redhat.com/archives/fedora-packaging/2009-May/msg00112.html


> 2) the format security patch should be submitted upstream
> [http://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment]
- Will do.

> 3) permissions of doc/ex/images/logo400x83.gif can be fixed too
Oh, rpmlint did not catch this :( Will update, thank you.


> 4) what is the %{nil} after the rm command good for? (in %install section)
%{nil} is expanded as nothing. No need to write this, however without this,
I have to write
 rm -rf \
    .gitignore .travis.yml \
    Doxyfile Gemfile Rakefile \
    before_*.sh \
    doc/.cvsignore \
    *.gemspec \
    test/ \
    ext/ \
<blank line>
popd

or
 rm -rf \
    .gitignore .travis.yml \
    Doxyfile Gemfile Rakefile \
    before_*.sh \
    doc/.cvsignore \
    *.gemspec \
    test/ \
    ext (no \ here)
popd
... which is inconvenient when I want to add some other files after
ext/ entry (well, not a big deal anyway)

-- 
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





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]