[Bug 558685] Review Request: gloobus-preview - A file previewer for

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

Rich Mattes <richmattes at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richmattes at gmail.com

--- Comment #1 from Rich Mattes <richmattes at gmail.com> 2010-01-27 16:40:51 EST ---
Quick comments on the specfile:

- You can use "find . -name '*.cpp' -exec chmod -x {} \;" to chmod the source
files, instead of typing them all out manually.  You can repeat this for header
files, etc.  It will make your spec more legible and save you from having to
add/remove files from the list as the program evolves.

- In the %files section, using the line %{_libdir}/globus will assign ownership
of that directory to your package, and install all of the files within it.  You
don't have to specify the %dir, and then all of the libraries within it.  The
same goes for %{datadir}/gloobus, you can install the directory and the images
within it with one line.  If there's some files you don't want to include, you
can use %exclude

- Use of macros is inconsistent, some lines use %{name} and some use gloobus. 
Pick one or the other to make your spec more clear

- If you're not installing libraries into %{_libdir}, you don't need to run
ldconfig.

- You should specify what your patch does, "all-mods" isn't really descriptive.
 Breaking your patches up is also a good idea, less maintenance for you if
some/all of the changes get merged upstream.  If applicable, you should also
supply a link to the upstream bug/patch

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