[Bug 511895] Review Request: clutter-imcontext - IMContext Framework Library for Clutter

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





--- Comment #4 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-07-19 15:58:35 EDT ---
(In reply to comment #3)
> > Here is my review for this package. Most are minor things but there is a
> > license issue.
> 
> Thanks
> 

You're welcome. I noticed one additional issue that I missed in the first pass:
The ./autogen.sh script is calling ./configure, which is redundant since you
will be calling it properly via %configure.

So, we should remove the first of this duplicate call by something like
   sed -i '/configure/d' autogen.sh
in %prep.

> From looking I couldn't see exactly what you meant by this. Further details
> would be great.
> 

Sure. For instance, you have these lines in the specfile:
   ...
   Files for development with %{name}.
   ...
   %{_libdir}/libclutter-imcontext-0.1.so.0
   ...
   %{_includedir}/clutter-imcontext-0.1

My understanding of macro consistency is that the above should be
   ...
   Files for development with %{name}.
   ...
   %{_libdir}/lib%{name}-0.1.so.0
   ...
   %{_includedir}/%{name}-0.1
or
   ...
   Files for development with clutter-imcontext.
   ...
   %{_libdir}/libclutter-imcontext-0.1.so.0
   ...
   %{_includedir}/clutter-imcontext-0.1

Btw, you can also save some time when you are doing updates by variable-izing
"0.1" by something like
   %global majorver 0.1
on top of the specfile. Then you can use %majorver everywhere else. Just a
suggestion.

> > ? Do we need to package the Makefile* stuff in %doc? One last suggestion: 
> > You could use "%doc doc/*" instead of just "%doc doc/" to avoid an extra
> > subdirectory.  
> 
> I'll look at why the Makefiles are there and if there's specific docs that can
> be generated will look at getting that done during the build.
> 

I checked the configure script. We can pass a --enable-gtk-doc flag to
%configure to actually build these documents in human readable gtk-doc/html
format. What we have done by "%doc doc/" is just packaging the raw
documentation, which is not that useful.

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