On Fri, Jul 16, 2004 at 12:37:08AM +0100, Max Bowsher wrote:

Thanks for taking care of this.  For my part, I think your patch is appropriate
for the next RC and in turn for 1.0.  It establishes an Autoconf API that should
serve APR well.  I have included a few technical suggestions.

(Regarding whether to make the arguments to APR_FIND_APR mandatory, I don't have
a solid opinion.)

> +  if test -z "$4"; then
> +    AC_MSG_ERROR([the APR_FI@&[EMAIL PROTECTED] macro requires an 
> acceptable-majors parameter])
> +  fi

You could have used an m4 conditional and ``errprint'' to raise this error at
autoconf-time.

What is that @&t@ business?

> +      case $apr_bundled_major in
> +        "")
> +          AC_MSG_ERROR([failed to find major version of bundled APR])
> +        ;;
> +        0)
> +          apr_temp_apr_config_file="apr-config"
> +        ;;
> +        *)
> +          apr_temp_apr_config_file="apr-$apr_bundled_major-config"
> +        ;;
> +      esac

Would this make more sense as follows?

case $apr_bundled_major in
     0)
        apr_temp_apr_config_file="apr-config"
        ;;
     [[:digit:]])
        apr_temp_apr_config_file="apr-$apr_bundled_major-config"
        ;;
     *)
        AC_MSG_ERROR([failed to find major version of bundled APR])
        ;;
esac

Again, thanks.

Reply via email to