(New patch set uploaded.)
On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda <ken...@google.com> wrote:
>
>
> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey <jeffbai...@google.com>wrote:
>
>> *sigh* It looks like the version at appspot.com isn't GA+ enabled, so I
>> sign in and it thinks I'm not signed in.
>> Anyhow, a few comments:
>>
>> Since it's generated by configure.ac, do you need it in bin_SCRIPTS? I
>> think that might cause it to get looked at twice.
>>
>
> The purpose of putting it in bin_SCRIPTS is to make sure that it is
> installed, which configure is not going to do automatically. The automake
> docs say that bin_SCRIPTS are by default not included in the dist, which is
> what we want here (since configure generates it).
>
>
>> You should pretty much always do a set -e at the top of a shell script to
>> catch errors early on.
>>
>
> Oops, fixed.
>
>
>>
>> *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
>> then
>>
>> Should those all be @pre...@?
>>
>
> Yes. :/
>
>
>> Also, I think test -a might be a bashism in this case.
>>
>
> Changed to "&& test".
>
>
>> Same for this line:
>>
>>
>> *79* if test $full_library = true -o $explicit_library = false; then
>>
>
> Done.
>
> Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
> traditionally separate. Not sure why gtk-config itself does not do this.
>
> Also also, I expanded the help text.
>
> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
> doubt anyone will correctly parse it otherwise (since people will code
> against official releases which have no suffix).
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/protobuf?hl=en
-~----------~----~----~----~------~----~------~--~---
