Re: [PATCH] strbuf.h: format according to coding guidelines

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 28, 2018, 20:11 UTC
Message-ID: <xmqqin2pbcwh.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqr2hdbdp8.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 12 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> So let's format strbuf.h in a way that we'd like to see:
>> * omit the extern keyword from function declarations
>
> OK
>
>> * name all parameters (usually the parameters are obvious from its type,
>>   but consider exceptions like
>>   `int strbuf_getwholeline_fd(struct strbuf *, int, int);`
>
> I am mildly against giving names to obvious ones.

Just to make sure nobody listening from sidelines do not misunderstand, ones like getwholeline_fd() that takes more than one parameter with the same time is a prime example of a function that take non-obvious parameters that MUST be named.

What I am mildly against is the rule that says "name ALL parameters". I tend to think these names make headers harder to read, and prefer to keep them to the minimum.

I actually do not mind the rule to be more like

 * Use the same parameter names used in the function declaration when
   the description in the API documentation refers the parameter.

That _forces_ people to write

	/**
	 * Read a whole line up to the terminating character 
	 * TERM (typically LF or NUL) from the file descriptor FD
	 * and replace the contents of the strbuf SB with the
	 * result ...
	 */
	int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term);

which is mostly equivalent to having a rule to always name all parameters, while still allowing "sb" to be omitted by rephrasing "the contents of the given strbuf with the result ..." and I consider that a good flexibility to have.