[Bug 528949] Review Request: oflb-Widelands-fonts - Widelands font

[bugzilla at redhat.com]

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





--- Comment #4 from Nicolas Mailhot <nicolas.mailhot at laposte.net>  2009-10-14 16:29:13 EDT ---
That being said, let's do a more thorough review

1. when creating a new file (such as the fontconfig rules one) it's better to
use SourceX instead of patch, that produces a simple patch in the vcs people
can look at or import. Using patch only adds lots of not-really useful +s
before each line

2. likewise for simple operations such as re-wrapping text files it's better to
script them instead of creating a trivial patch that will need to be rebased
every few releases

I often use something like

for txt in *.txt; do
   fold -s $txt > $txt.new &&\
   sed -i 's/\r//' $txt.new &&\
   touch -r $txt $txt.new &&\
   mv $txt.new $txt
done

(you can remove the sed if you don't need to fix DOS EOLs, and add an iconv if
necessary)

3. We sadly do not seem to have a standard script to invoque fontforge. So I
can't say if your way is better or worse than others. It's probably a good idea
to write a small makefile and get it included upstream so upstream at least
will check it's happy with the way we generate the font. Someday we'll need to
codify this so all our packages have the same bugs :)

4. you can put multiple files on a single %doc line

The rest seems fine (but I'll need a package buildable in rawhide to run our
full battery of tests

Thank you for submitting this package, it's not perfect but it's also not too
bad and you were awfully quick :)

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