[Bug 729512] Review Request: graphite2 - Font rendering capabilities for complex non-Roman writing systems

bugzilla at redhat.com bugzilla at redhat.com
Thu Aug 18 08:17:04 UTC 2011


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

Martin Gieseking <martin.gieseking at uos.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |martin.gieseking at uos.de
         AssignedTo|nobody at fedoraproject.org    |martin.gieseking at uos.de
               Flag|                            |fedora-review?

--- Comment #6 from Martin Gieseking <martin.gieseking at uos.de> 2011-08-18 04:17:03 EDT ---
(In reply to comment #5)
> I think following lines
> cp LICENSE $RPM_BUILD_ROOT/usr/share/graphite2
> 
> %files
> %doc /usr/share/graphite2/LICENSE
> 
> should be replaced with just
> 
> %files
> %doc LICENSE

Yes, definitely. There's no need to install the doc files. Just add them with
%doc by giving their relative location in the directory tree of the extracted
tarball.

Also, add the files COPYING and ChangeLog to the base package.


> Also this line:
> %doc /usr/share/graphite2/*.cmake
> 
> seems incorrect to me. Assuming *.cmake belongs to %doc (which I don't know)
> and therefore package can run without those files - is it neccessary to use the
> whole path? At least I would replace /usr/share with the macro (and graphite2
> for that matter).

Yes. Just drop "%doc" and replace /usr/share with %{_datadir}. 


Some additional notes:
- The devel package must require the base package as mentioned here:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

- You can drop Requires: pkgconfig as you probably don't want to build the
  package for EPEL < 6. The dependency on pkgconfig is detected automatically
  in Fedora and EPEL 6.

- Please be a bit more verbose in %files, i.e. try to avoid adding files
  with plain asterisks, especially, if only few files are addressed:
  %{_bindir}/comparerenderer
  %{_bindir}/gr2fonttest 
  %{_libdir}/libgraphite2.so.*
  %{_libdir}/libgraphite2.so
  %{_libdir}/pkgconfig/graphite2.pc
  %{_datadir}/graphite2/graphite2*.cmake
  etc.

  This helps to get a better overview of what's actually going to be added,
  and avoids packaging unwanted files.

- Add %dir %{_datadir}/graphite2/ to the devel package for proper directory
  ownership.

- Either add %defattr(-,root,root,-) to the base package or drop it from the 
  devel package. Both is fine but you should use it consistently.

- "make docs" in %build has no effect at the moment because of the missing
  BRs doxygen, tex(latex), and asciidoc. Add them and spread the generated
  documentation on the base and devel package (using %doc).

- Please ask upstream to update the FSF address in the source files and
  COPYING.

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



More information about the package-review mailing list