Re: [PATCH v3] Add Documentation/CodingGuidelines

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2007, 23:14 UTC
Message-ID: <7vtznx8x86.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0711072233010.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> new file mode 100644
> index 0000000..2d8656f
> --- /dev/null
> +++ b/Documentation/CodingGuidelines
> @@ -0,0 +1,106 @@
> +As a popular project, we also have some guidelines to keep to the
> +code.  For git in general, two rough rules are:

I'd rather not to say "As a popular project" here. That is something for others to decide, not for us to advertise.

Also we now have three rules here ;-).

> +
> + - Most importantly, we never say "It's in POSIX; we'll happily
> +   ignore your needs should your system that does not conform."

"should your system not conform"? "if your system does not conform"?

> +As for more concrete guidelines, just imitate the existing code
> +(this is a good guideline, no matter which project you are contributing
> +to...).  But if you must have some list of rules, here they are.

s/\.\.\.//;

Show 9 quoted lines

> +For C programs:
> +
> + - Use tabs to indent, and interpret tabs as taking up to 8 spaces
> +
> + - Try to keep to at most 80 characters per line
> +
> + - When declaring pointers, the star sides with the variable name, i.e.
> +   "char *string", not "char* string" or "char * string".  This makes
> +   it easier to understand "char *string, c;"

End each sentence with a full stop "." for consistency.

Show 12 quoted lines

> +
> + - Do not use curly brackets unnecessarily.  I.e.
> +
> +	if (bla) {
> +		x = 1;
> +	}
> +
> +   is frowned upon.  A gray area is when the statement extends over a
> +   few lines, and/or you have a lengthy comment atop of it.  Also,
> +   like in the Linux kernel, if there is a long list of "else if"
> +   statements, it can make sense to add curly brackets to single
> +   line blocks.

As Robin suggests, s/Do not use/Avoid using/ feels more in line with the spirit with the "A gray area is..." description.

I think the official name for {} are "braces", by the way.

Show 6 quoted lines

> +   Double negation is often harder to understand than no negation at
> +   all.
> +
> +   Some clever tricks, like using the !! operator with arithmetic
> +   constructs, can be extremely confusing to others.  Avoid them,
> +   unless there is a compelling reason to use them.

Need a bullet point "-" before "Double" (but not "Some").

> + - #include system headers in git-compat-util.h.  Some headers on some
> +   systems show subtle breakages when you change the order, so it is
> +   best to keep them in one place.
> +

The first #include in C files except platform specific compat/ implementation should be git-compat-util.h or another header file that includes it, such as cache.h and builtin.h.

> + - if you are planning a new command, consider writing it in shell or
> +   perl first, so that changes in semantics can be easily changed and
> +   discussed.  Many git commands started out like that, and a few are
> +   still scripts.

Begin a statement with a Capital letter, for consistency.