[for review] HTMLized docs in compiler/notes
Bert THOMPSON
aet at cs.mu.oz.au
Wed Apr 2 20:33:31 AEST 1997
Tyson,
Please review the following.
BTW, if people are happy to nuke the plain text files and retain the HTML,
thus eliminating the dual update problem, I'll nuke.
(By `nuke', I mean put in the attic, of course.)
Bert
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Estimated hours taken: 1
Translated CODING_STANDARDS and REVIEWS to HTML. This obviously creates
a dual update problem. We should solve this by putting CODING_STANDARDS
and REVIEWS in the attic and retaining CODING_STANDARDS.html and
REVIEWS.html.
Also added CODING_STANDARDS.html and REVIEWS.html to the Mmakefile
so they are installed on the web pages.
The other documents in compiler/notes will be HTMLized soon.
compiler/notes/
Mmakefiles
CODING_STANDARDS.html
REVIEWS.html
CVS: ----------------------------------------------------------------------
CVS: Enter Log. Lines beginning with `CVS: ' are removed automatically
CVS:
CVS: Committing in .
CVS:
CVS: Modified Files:
CVS: Mmakefile
CVS: Added Files:
CVS: CODING_STANDARDS.html REVIEWS.html
CVS: ----------------------------------------------------------------------
Index: Mmakefile
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/notes/Mmakefile,v
retrieving revision 1.2
diff -c -r1.2 Mmakefile
*** 1.2 1997/04/02 09:07:12
--- Mmakefile 1997/04/02 09:52:49
***************
*** 16,22 ****
DOCS_TEXT=ALLOCATION AUTHORS CODING_STANDARDS COMPILER_DESIGN \
GC_AND_C_CODE GLOSSARY MODULE_SYSTEM RELEASE_CHECKLIST \
REVIEWS TODO
! DOCS_HTML=
DOCS_ALL=$(DOCS_TEXT) $(DOCS_HTML)
#-----------------------------------------------------------------------------#
--- 16,22 ----
DOCS_TEXT=ALLOCATION AUTHORS CODING_STANDARDS COMPILER_DESIGN \
GC_AND_C_CODE GLOSSARY MODULE_SYSTEM RELEASE_CHECKLIST \
REVIEWS TODO
! DOCS_HTML=CODING_STANDARDS.html REVIEWS.html
DOCS_ALL=$(DOCS_TEXT) $(DOCS_HTML)
#-----------------------------------------------------------------------------#
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
<html>
<head>
<title>
Mercury Coding Standard for the Mercury Project
</title>
</head>
<body
bgcolor="#ffffff"
text="#000000"
>
<hr>
<!-------------------------->
<h1>
Mercury Coding Standard for the Mercury Project</h1>
<hr>
<!-------------------------->
<h2> Use of language features </h2>
<p>
Language features that do not work in SICStus Prolog
(including functions, reverse modes of arithmentic,
the NU-Prolog syntax for negation and if-then-else,
and code requiring mode reordering) should not be used
in the Mercury compiler --- at least not until we
have implemented a proper debugger.
<p>
<h2> Documentation </h2>
<p>
Each module should contain header comments which state
the module's name, main author(s), and purpose, and
give an overview of what the module does, what are the
major algorithms and data structures it uses, etc.
<p>
Each type or predicate that is exported from a module should have
sufficient documentation that it can be understood without reference
to the module's implementation section.
<p>
Each predicate other than trivial access predicates should have
a short comment describing what the predicate is supposed to do,
and what the meaning of the arguments is.
<p>
There should be a comment for each field of a structure saying
what the field represents.
<p>
Any user-visible changes such as new compiler options or new
features should be documented in appropriate section
of the Mercury documentation (usually the Mercury User's Guide
and/or the Mercury Reference Manual). Any major new features
should be documented in the NEWS file, as should even small
changes to the library interface, or anything else that might
cause anyone's existing code to break.
<p>
Any new compiler modules or other major design changes should
be documented in `compiler/notes/COMPILER_DESIGN'.
<p>
<h2> Naming </h2>
<p>
Variables should always be given meaningful names,
unless they are irrelevant to the code in question.
For example, it is OK to use single-character names
in an access predicate which just sets a single
field of a structure, such as
<pre>
bar_set_foo(Foo, bar(A, B, C, _, E), bar(A, B, C, Foo, E)).
</pre>
Variables which represent different states or different versions
of the same entity should be named Foo0, Foo1, Foo2, ..., Foo.
<p>
Predicates which get or set a field of a structure or ADT should
be named bar_get_foo and bar_set_foo respectively, where bar
is the name of the structure or ADT and foo is the name of the field.
<p>
<h2> Coding </h2>
<p>
Your code should make as much reuse of existing code as possible.
"cut-and-paste" style reuse is highly discouraged.
<p>
Your code should be efficient. Performance is a quite
serious issue for the Mercury compiler.
<p>
No fixed limits please!
(If you really must have a fixed limit, include detailed documentation
explaining why it was so hard to avoid.)
<p>
<h2> Error Handling </h2>
<p>
Code should check for both erroneous inputs from the user and also
invalid data being passed from other parts of the Mercury compiler.
You should also always check to make sure that the routines that you
call have succeed; make sure you don't silently ignore failures. (This
last point almost goes without saying in Mercury, but is particularly
important to bear in mind if you are writing any C code or shell
scripts, or if you are interfacing with the OS.)
<p>
Calls to error/1 should always indicate an internal software error, not
merely incorrect inputs from the user, or failure of some library routine
or system call.
<p>
Error messages should follow a consistent format. For compiler error
messages, each line should start with the source file name and line
line number in "%s:%03d: " format (but use prog_out__write_context --
we may want to change this format later, e.g. to include column
numbers). Compiler error messages should be complete sentences; they
should start with a capital letter and end in a full stop. For error
messages that are spread over more than one line (as are most of them),
the second and subsequent lines should be indented two spaces. If the
`--verbose-errors' option was set, you should print out additional text
explaining in detail what the error message means and what the likely
causes are.
<p>
Error messages from the runtime system should begin with the text
"Mercury Runtime:", preferably by using the fatal_error() routine.
<p>
If a system call or C library function that sets errno fails, the
error message should be printed with perror() or should contain
strerror(errno). If it was a function manipulating some file, the
error message should include the filename.
<p>
<h2> Layout </h2>
Code should be indented consistently.
Tabs should be every 8 spaces.
Indentation should be one tab per level of indentation.
except for highly intended code which may be
indented at 4 spaces per level of indentation.
Each line should not extend beyond 79 characters,
unless this is necessary due to the use of long string literals.
If-then-elses should always be parenthesized,
except that an if-then-else that occurs as the else
part of another if-then-else doesn't need to be parenthesized.
The semicolon of a disjunction should never be at the
end of a line -- put it at the start of the next line instead.
The condition of an if-then-else can either be on the same
line as the opening parenthesis and the `->',
<pre>
( test1 ->
goal1
; test2 ->
goal2
;
goal
)
</pre>
or, if the test is complicated, it can be on a line of its own:
<pre>
(
very_long_test_that_does_not_fit_on_one_line(VeryLongArgument1,
VeryLongArgument2)
->
goal1
;
test2a,
test2b,
->
goal2
;
test3 % would fit one one line, but separate for consistency
->
goal3
;
goal
).
</pre>
Type definitions should be formatted in the following style:
<pre>
:- type my_type
---> my_type(
some_other_type % comment explaining it
).
:- type some_other_type == int.
:- type foo
---> bar(
int, % comment explaining it
float % comment explaining it
)
; baz
; quux.
</pre>
If an individual clause is long, it should be broken into sections,
and each section should have a "block comment" describing what it does;
blank lines should be used to show the separation into sections.
Comments should precede the code to which they apply, rather than
following it.
<pre>
%
% This is a block comment; it applies to the code in the next
% section (up to the next blank line).
%
blah,
blah,
blahblah,
blah,
</pre>
If a particular line or two needs explanation, a "line" comment
<pre>
% This is a "line" comment; it applies to the next line or two
% of code
blahblah
</pre>
or an "inline" comment
<pre>
blahblah % This is an "inline" comment
</pre>
should be used.
At a higher level, code should be grouped into bunches of related
predicates, functions, etc., and sections of code that are conceptually
separate should be separated with dashed lines:
<pre>
%---------------------------------------------------------------------------%
Double-dashed lines, i.e.
%---------------------------------------------------------------------------%
%---------------------------------------------------------------------------%
</pre>
can also be used to indicate divisions into major sections.
<h2> Testing </h2>
<p>
Every change should be tested before being committed.
The level of testing required depends on the nature of the change.
If this change fixes an existing bug, and is not likely to introduce
any new bugs, then just compiling it and running some tests
by hand is sufficient. If the change might break the compiler,
you should run a bootstrap check (using the `bootcheck' script)
before committing. If the change means that old versions of
the compiler will not be able to compile the new version of the
compiler, you must notify all the other Mercury developers.
<p>
In addition to testing before a change is committed, you need to make
sure that the code will not get broken in the future by adding tests to
the test suite. Every time you add a new feature, you should add some
test cases for that new feature to the test suite. Every time you fix
a bug, you should add a regression test to the test suite.
<p>
<h2> Committing changes </h2>
<p>
Before committing a change, you should get someone else to review your
changes.
<p>
The file "REVIEWS" contains more information on review policy.
<hr>
<!-------------------------->
Last update was $Date: 1997/04/02 05:57:58 $ by $Author: aet $@cs.mu.oz.au. <br>
</body>
</html>
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
<html>
<head>
<title>
Reviews
</title>
</head>
<body
bgcolor="#ffffff"
text="#000000"
>
<hr>
<!----------------------->
<h1> Reviews </h1> <p>
This file outlines the policy on reviews for the Mercury system.
<hr>
<!----------------------->
<h2> Reviewable material </h2>
<p>
All changes to the Mercury repository, including the compiler,
documentation, www pages, library predicates, runtime system, and tools
need to be reviewed.
<p>
<h2> Review process </h2>
<ol>
<li> Make sure you are working with an up-to-date copy of the
module you are using.
<li> If change is a code change, test change. See "Testing" section
of coding standards. Testing may take time - don't forget
that steps 3, 4 and 5 can be done in parallel.
<li> Create diff - use `cvs diff -u'. New files should be
appended verbatim to the end of the diff, with descriptions
indicating the name of the file.
<li> Write log message for this change - use template (see below).
<li> Review diff and log message yourself. (see below)
<li> Send to mercury-developers at cs.mu.oz.au, with the subject
"cvs diff: <short description of change>".
Nominate a reviewer at top of diff (see below).
<li> Wait for review (see below).
<li> Fix any changes suggested.
<li> Repeat above steps until approval.
<li> Commit change (see below).
</ol>
<h2> Log Messages </h2>
Use the template that cvs provides.
<pre>
Estimated hours taken: _____
<overview or general description of changes>
<directory>/<file>:
<detailed description of changes>
</pre>
In estimated hours, include all your time to fix this problem -
including debugging time.
<p>
The description should state why the changes were made, not just what
the changes were. All file modifications related to the same change
should be committed together, and use the same log message, even over
multiple directories. The reason for this is that the log messages can
be viewed on a file-by-file basis, and it is useful to know that a small
change of a file in a subdirectory is related to a larger change in
other subdirectories.
<p>
For very small changes, the <overview or general description> can be
omitted, but the <detailed description> should stay.
<p>
If adding a new feature, this is a good place to describe the feature,
how it works, how to turn it on and off, and any present limitations of
the feature (note that all this should also be documented within the
change, as well). If fixing a bug, describe both the bug and the fix.
<p>
<h2> Self-Review </h2>
<p>
You should also review your own code first, and fix any obvious
mistakes. Where possible add documentation - if there was something you
had to understand when making the change, document it - it makes it
easier to review the change if it is documented, as well as generally
improving the state of documentation of the compiler.
<p>
<h2> Review </h2>
<p>
We're now posting all diffs to mercury-developers at cs.mu.oz.au.
<p>
The reasons for posting to mercury-developers are:
<ul>
<li> To increase everyone's awareness of what changes are taking
place.
<li> Give everyone interested a chance to review your code, not
just the reviewer. Remember, your changes may impact upon
the uncommitted work of others, so they may want to give
input.
<li> Allow other people to read the reviewer's comments - so the same
problems don't have to be explained again and again.
<li> People can try to see how your changes worked without having
to figure out how to get cvs to generate the right set of
diffs.
<li> Important decisions are often made or justified in reviews, so
these should be recorded.
</ul>
You should try to match the reviewer to the code - someone familiar with
a section of code can review faster and is more likely to catch errors.
Put a preamble at the start of your diff to nominate who you would like
to review the diff.
<p>
<h2> Waiting and approval </h2>
<p>
Waiting for approval need not be wasted time. This is a good time to
start working on something else, clean up unused workspaces, etc. In
particular, you might want to run long running tests that have not yet
been run on the your change (different grades, different architectures,
optimisation levels, etc).
<p>
The reviewer(s) should reply, indicate any problems that need to be
corrected, and whether the change can be committed yet. Design issues
may need to be fully justified before you commit. You may need to fix
the problems, then go through another iteration of the review process,
or you may be able to just fix a few small problems, then commit.
<p>
<h2> Committing </h2>
If you have added any new files or directories, then before committing
you must check the group-id and permissions of the newly created files
or directories in the CVS repository. Files should be readable by
group mercury and directories should be both readable and writable by
group mercury. (Setting of permissions will be enforced by the
pre-commit check script `CVSROOT/check.pl'.)
<p>
Use the log message you prepared for the review when committing.
<p>
<h2> Exceptions: Commit before review </h2>
<p>
The only time changes should be committed before being reviewed is when they
satisfy all of the following conditions:
<ul>
<li> (a) the change is simple
<li> (b) you are absolutely sure the change will not introduce bugs
<li> (c) you are sure that the change will pass review with only
trivial corrections (spelling errors in comments, etc.)
<li> (d) there are no new design decisions or changes to previous
design decisions in your change (the status quo should
be the default; you must convince the reviewer(s) of
the validity of your design decisions before the code
is committed).
<li> (e) you will be around the next day or two to fix the bugs
that you were sure could never happen
<li> (f) committing it now will make life significantly easier
for you or someone else in the group
</ol>
<p>
If the compiler is already broken (i.e. it doesn't pass it's nightly
tests), and your change is a bug fix, then it's not so important to be
absolutely sure that your change won't introduce bugs. You should
still be careful, though. Make sure you review the diffs yourself.
<p>
Similarly, if the code you are modifying is a presently unused part of
code - for example a new feature that nobody else is using, that is
switchable, and is switched off by default, or a new tool, or an `under
development' webpage that is not linked to by other webpages yet, the
criteria are a bit looser. Don't use this one too often - only for
small changes. You don't want to go a long way down the wrong track
with your new feature, before finding there's a much better way.
<p>
If these conditions are satisfied, then there shouldn't be any problem
with mailing the diff, then committing, then fixing any problems that
come up afterwards, provided you're pretty sure everything will be okay.
This is particularly true if others are waiting for your work.
<p>
<h2> Exceptions: No review </h2>
<p>
The only time changes should be committed without review by a second
person is when they satisfy all of the following conditions:
<ul>
<li> (a) it is a very small diff that is obviously correct <br>
eg: fix typographic errors <br>
fix syntax errors you accidently introduced <br>
fix spelling of people's names <br> <p>
These usually don't need to be reviewed by a second
person. Make sure that you review your own changes,
though. Also make sure your log message is more
informative than "fixed a typo", try "s/foo/bar" or
something so that if you did make a change that people
don't approve of, at least it's seen quickly.
<li> (b) it is not going to be publically visible <br>
eg: Web pages, documentation, library, man pages. <p>
Changes to publically visible stuff should always be
reviewed. It's just too easy to make spelling errors,
write incorrect information, commit libel, etc. This
stuff reflects on the whole group, so it shouldn't be
ignored.
</ul>
<hr>
<!-------------------------->
Last update was $Date: 1997/04/02 05:57:58 $ by $Author: aet $@cs.mu.oz.au. <br>
</body>
</html>
More information about the developers
mailing list