[Bug 472791] Review Request: fontbox - A Java library for parsing font files

[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=472791





--- Comment #3 from Jerry James <loganjerry at gmail.com>  2008-12-17 13:27:20 EDT ---
Here are a few things I noticed on an initial read-through of the spec file. 
First, the Java packaging guidelines state that Java packages must
"BuildRequires: java-devel" and "Requires: jpackage-utils" (see
https://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires), both
of which are missing.

Second, I don't understand why you are using a half-maven half-ant build. 
Shouldn't you go with one or the other?

Third, the GCJ guidelines have not been followed (see
https://fedoraproject.org/wiki/Packaging/GCJGuidelines).  Is there some reason
for this?

Fourth, the use of dos2unix is unnecessary.  You can accomplish the same result
as follows:

sed -i -e 's/\r//g' <list of files>

Since sed is already in the default set of packages, this does not lead to any
BuildRequires.  There is no point in fixing the line endings of javadoc files
since you are regenerating those files in the %build section anyway.  Also, the
list of files is too inclusive: dos2unix is being invoked on .pdf, .jpg, .png,
.gif, and Thumbs.db files.  Those are binary formats, so dos2unix is very
probably corrupting them.  Also, since the PDF files are just simple
transformations of the HTML files, I doubt they add any value.  They're
extremely short and not linked to one another, so I don't see the point in
including them.

Fifth, changing the encodings of the XML files in docs/skin is not sufficient,
since they use XML encoding declarations at the top:

<?xml version="1.0" encoding="ISO-8859-1"?>

which means that we are now messing up any XML processors because they are
really getting UTF-8 encoded files.  This won't matter for the English file,
because it uses only ASCII, which is a subset of both UTF-8 and ISO-8859-1. 
However, the German, Spanish, and French versions all use non-ASCII characters,
so it will matter for them.  If you really need to change the encoding, I
recommend making a patch that both changes the encoding and changes the XML
encoding declaration.

MUST items:
- Output of rpmlint:
2 packages and 1 specfile checked; 0 errors, 0 warnings.
- Package name: OK
- Spec file name: OK
X Packaging guidelines: see the list of items above
- Licensing guidelines: OK
- License field matches: OK
- Text license file in %doc: OK
- Spec file in American English: OK
- Spec file is legible: OK
- Sources match upstream: OK (checked with md5sum)
- Compiles into binary RPMs on at least one platform: OK (checked on x86_64)
- Use of ExcludeArch: OK (I did not check other arches, but this is noarch)
X All build dependencies in BuildRequires: need to add java-devel; see above
- Proper locale handling: OK
- ldconfig: OK
- Relocatable package: OK
- Own all created directories: OK
- No duplicate entries in %files: OK
- Proper file permissions: OK
- %clean section: OK
- Consistent use of macros: OK
- Code or permissible content: OK
- Large documentation: OK (total documentation size is 424K)
- Nothing in %doc affects runtime: OK
- Header files in -devel: OK
- Static libraries in -static: OK
- Pkgconfig files: OK
- .so files in -devel: OK
- -devel package requires main package: OK
- No libtool archives: OK
- Desktop file: OK
- Don't own files/directories owned by other packages: OK
- Clean buildroot in %install: OK
- Filenames are UTF-8: OK

SHOULD items:
- Query upstream for missing license file: OK
- Description and summary translations: OK
- Package builds in mock: OK (checked for F-10 x86_64 only)
- Builds on all supported architectures: did not check
- Package functions as described: did not check
- Sane scriptlets: see comments about maven & ant above
X Subpackages require the base package: NO, the javadoc package does not
require the main package
- Placement of pkgconfig files: OK
- File dependencies: OK

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