for review: fixing bart's complaints about MERCURY_OPTIONS

Zoltan Somogyi zs at cs.mu.OZ.AU
Mon Aug 10 14:34:27 AEST 1998


> Not knowing what "reviewing a change" means within the
> mercury-developers, I read through what you send; skipped the stuff
> that had no meaning for me and arrived at the following petty
> comments:

Reviewing a change means

- checking whether you understand everything in the change; if you don't,
  ask for clarification;

- checking whether you agree that the change is correct, i.e. does not
  introduce new bugs and that (a) if the change is a new feature, the feaure
  works (which usually requires a test case), or (b) if the change is a bug
  fix, the changes fixes all occurrences of the bug.

A review may also contain suggestions for material, performance or stylistic
improvements.

BTW, what was the stuff without meaning?

> + at c @item --heap-zone-size=@var{size}
> + at c Sets the size of the redzone on the heap to @var{size} kilobytes.
> 
> since you are being verbose already, why not as option:
> 
>         --heap-redzone-size=@var{size}

Done.

> the current user's guide doesn't mention "redzone" (unless I
> overlooked it)

That's deliberate. The options that control the redzone are for implementors
of Mercury only. They are not documented in user-accessible places (the @c
is the texinfo comment character).

> maybe it is worth mentioning somewhere why someone might want to set a
> redzone size and what the effect is ?

The reason why the redzone is not documented is that this explanation
must be too low level, and would confuse rather than help non-implementors.

> no idea how much work this would be ... if the user gave as
> non-existing option "blabla", then a message like:
> 
>         The MERCURY_OPTIONS environment variable
>         contains "blabla" which is not an option.
>         Please refer ...
> 
> is more user friendly.

OK, done.

See, that's two respects in which your comments have improved the change.
Maybe the HAL group should adopt reviews as well :-)

Zoltan.



More information about the developers mailing list