[m-rev.] for review: --disable-deep-profiler
Peter Ross
peter.ross at miscrit.be
Wed Jun 6 00:16:31 AEST 2001
I have taken your suggestions and disabled the deep profiler by default and
swapped the sense of the variable to ENABLE_DEEP_PROFILER.
Pete
Fergus wrote:
> On 05-Jun-2001, Peter Ross <peter.ross at miscrit.be> wrote:
>
> Mmake.common.in:
> > +# Do we disable building of the deep profiler.
> > +DISABLE_DEEP_PROFILER=@DISABLE_DEEP_PROFILER@
>
> The comment here should say what the allowed values are (`yes' or `no').
>
> It would be slightly better to have the variable be the positive sense
> (i.e. ENABLE_DEEP_PROFILER) rather than the negative sense.
> In general using positive variable names rather than negative ones
> avoids double negations, which makes things easier to understand.
>
> Apart from anything else, you'd save a few lines of code:
>
> > +# Add an option that disables the deep profiler.
> > +#
> > +AC_ARG_ENABLE(deep-profiler,
> > + [ --disable-deep-profiler disable the deep profiler],
> > + mercury_cv_enable_deep_profiler="$enableval",
> > + mercury_cv_enable_deep_profiler=yes
> > +)
> > +if test "$mercury_cv_enable_deep_profiler" = "no"; then
> > + DISABLE_DEEP_PROFILER=yes
> > +else
> > + DISABLE_DEEP_PROFILER=no
> > +fi
> > +AC_SUBST(DISABLE_DEEP_PROFILER)
>
> The five lines in the middle could be replaced by just
>
> ENABLE_DEEP_PROFILER=$mercury_cv_enable_deep_profiler
>
> Also, the deep profiler should not be enabled by default; it should
> only be enabled by default on platforms that support the Posix features
> that it uses. If there isn't code yet to check for those features,
> then I think it would be better to leave it disabled by default.
>
> --
> Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the
pursuit
> | of excellence is a lethal habit"
> WWW: <http://www.cs.mu.oz.au/~fjh> | -- the last words of T. S. Garp.
> --------------------------------------------------------------------------
> mercury-reviews mailing list
> post: mercury-reviews at cs.mu.oz.au
> administrative address: owner-mercury-reviews at cs.mu.oz.au
> unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message:
unsubscribe
> subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message:
subscribe
> --------------------------------------------------------------------------
>
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list