Stephen Gallagher wrote:
On 02/01/2010 07:11 PM, Dmitri Pal wrote:
> Stephen Gallagher wrote:
>> Patches 0001-0003: License and package libpath_utils
>>
> 01: In the header should the COPYING.lesser be COPYING.LESSER?
Otherwise Ok.
I will correct that.
> 02: Ok
> 03: I can' review this file I do not know the RPM syntax yet.
>> Patches 0004-0007: License and package libcollection. (Patch 0005 fixes
>> an incorrect #include in collection.h)
>>
> 04: Seems Ok, but now I am confused. I thought we had both licenses on
> the level above in "common", is this not enough? I thought we
> investigated that this is enough. What has changed. Why we put them in
> all places?
After more consideration, I realized that it is necessary to have the
licenses in the root of any location where we run "configure", because
it is possible to generate a tarball at that time. If we were to create
a libcollection-only tarball, it needs to have its license included as
well.
> 05: Is it just for cleanness of the interface? Seems ok if it compiles.
It's not for "cleanness of the interface". The trace.h header is not an
installed header. Requiring it in a public header means that no code
that builds against libcollection will compile.
Are you including trace.h into the library?
Without it the debug build will be broken.
It should be a part of the SRPM.
> 06: Seems Ok but I am not an expert. Second pair of eyes would not be
> redundant. Should the -version-info 1:0:0 match the versions in the
> configure.ac?
-version-info refers to the API version, not the program version. The
three numbers mean:
current
The most recent interface number that this library implements.
revision
The implementation number of the current interface.
age
The difference between the newest and oldest interfaces that this
library implements. In other words, the library implements all the
interface numbers in the range from number current - age to current.
If two libraries have identical current and age numbers, then the
dynamic linker chooses the library with the greater revision number.
This is what I suspected. Thanks for the explanation.
> 07: I can' review this file I do not know the RPM syntax
yet.
>> Patch 0008-0011: License and package libini_config. (Patch 0009
fixes an
>> array index mistake that was throwing a compiler warning and would have
>> been a potential memory-corruption bug.)
>>
> 08: same as 04.
> 09: good catch, full ack
> 10: same as 06
> 11: same as 07
>> Patch 0012-0014: License and package libref_array
> 12: same as 04
> 13: Seems Ok but I am not an expert. Second pair of eyes would not be
> redundant. Should the -version-info 1:0:0 match the versions in the
> configure.ac?
> 14: same as 07
> Bottom line.
> I do not see major issues with what I understand.
> I have a dirty tree so I did not apply the patches.
> If the patches apply, the version should not match (I suspect it should
> not because something tells me that version-info might define the
> version of the shared library interface) and someone checks the RPM
> patches I think there will be a joint ack.
>>
>> These are all very simple patches, there are just a bunch of them :) It
>> should be an easy review.
>>
> -------------------------
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/