[Bug 551258] Review Request: libgcal - A library to access google calendar events and contacts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #4 from Mario Ceresa <mrceresa@xxxxxxxxx> 2010-01-21 10:22:56 EST ---
Hello Scott, Andrea and Michael,
I'm very sorry to reply only now, but I've been out for work and had so little
time to focus on the review:

@Andrea: thanks a lot for pointing me to the right source!
@Michael: thanks for the comments:

> The base shared library still belongs into "System Environment/Libraries".
> You can put multiple files behind a single %doc.
> File "INSTALL" is irrelevant to RPM package users.

Fixed. Install added as a doc for development subpackage.

> Caution! Avoid '#comments' after scriptlet sections. They would be included as
> scriptlet bodies, which breaks -p /sbin/ldconfig for example. 

Thanks! This one was trubling me for a while but I'd never thought the comments
were the problem :)

> This description would benefit from some cleanup. Please make this a complete
> sentence in English.

Oops! I must have missed how ugly the previous sentence was. Hope now it's a
bit better

> Have you looked into enabling the built-in test-suite and creating a proper
%check section for it?
Yes, but I had some problems so thought to overlook. If it is really important
I could give it another shot... 

> The -devel packages depends on libxml and libcurl headers, so adding
>"Requires: libxml2-devel libcurl-devel" would be necessary for all targets that
>don't add automatic pkgconfig dependencies (Fedora 10 and older, EPEL 5 and
>older).
> If you want to rely on the automatic pkgconfig dependencies, you can drop the
> "Requires: pkgconfig".


I don't understand this: when is that I have to manually specify Requires and
when it's better to let them be picked up by rpm?


> The header files in libgcal-devel bear a huge risk of causing file conflicts.
> This is a blocker IMO. This library ought to install them into a libgcal
> sub-directory:

Ok I added two lines that solve the problems in the INSTALL section:

mkdir -p %{buildroot}/usr/include/libgcal
mv %{buildroot}/usr/include/*.h %{buildroot}/usr/include/libgcal/

but I'm not sure this is the best way to do it.

New files are:

http://mrceresa.fedorapeople.org/libgcal.spec
http://mrceresa.fedorapeople.org/libgcal-0.9.3-2.fc12.src.rpm

Mario

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]