[PATCH v2 0/5] Documentation: some coding guideline updates

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 7866 bytes --]

Hi,

this is the second version of my patch series that aims to improve our
coding guidelines such that we arrive at a more consistent coding style.

Changes compared to v1:

  - Fix clang-format to use a single space to indent preprocessor
    directives instead of using tabs. Thus, this series is now built
    with kn/ci-clang-format at 1b8f306612 (ci/style-check: add
    `RemoveBracesLLVM` in CI job, 2024-07-23) merged into v2.46.0.

  - Adapt the coding guidelines accordingly to also only use a single
    space for indentation of nested preprocessor directives.

  - Adopt a proposal by Junio to more clearly spell out the relationship
    between a subsystem `S`, `struct S` and its functions `S_<verb>()`.

  - Document `S_clear()`-style functions. I have adopted the proposal by
    Junio hear, where `clear = release + init` with the restriction that
    `S_init()` must not allocate any resources.

  - Add another patch on top that makes variable initializers consistent
    in our coding guidelines. Our style is to add spaces between the
    curly brace and the initializers (`struct foo bar = { something };`).

I think I captured everything that came out of the discussion, but
please let me know in case I misinterpreted or forgot anything.

Thanks!

Patrick

Patrick Steinhardt (5):
  clang-format: fix indentation width for preprocessor directives
  Documentation: clarify indentation style for C preprocessor directives
  Documentation: document naming schema for structs and their functions
  Documentation: document idiomatic function names
  Documentation: consistently use spaces inside initializers

 .clang-format                  |  6 +++--
 Documentation/CodingGuidelines | 48 +++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 3 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  c33ad700d6 clang-format: fix indentation width for preprocessor directives
1:  64e0b44993 ! 2:  e3baf01234 Documentation: clarify indentation style for C preprocessor directives
    @@ Metadata
      ## Commit message ##
         Documentation: clarify indentation style for C preprocessor directives
     
    -    There has recently been some discussion around how C preprocessor
    -    directives shall be indented [1]. This discussion was settled towards
    -    indenting after the hash by two spaces [2]. Document it as such.
    -
    -    [1]: https://lore.kernel.org/all/xmqqwmmm1bw6.fsf@gitster.g/
    -    [2]: https://lore.kernel.org/all/20240708092317.267915-2-karthik.188@gmail.com/
    +    In the preceding commit, we have settled on using a single space per
    +    nesting level to indent preprocessor directives. Clarify our coding
    +    guidelines accordingly.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ Documentation/CodingGuidelines: For C programs:
       - We use tabs to indent, and interpret tabs as taking up to
         8 spaces.
      
    -+ - Nested C preprocessor directives are indented after the hash by two
    -+   spaces per nesting level.
    ++ - Nested C preprocessor directives are indented after the hash by one
    ++   space per nesting level.
     +
     +	#if FOO
    -+	#  include <foo.h>
    -+	#  if BAR
    -+	#    include <bar.h>
    -+	#  endif
    ++	# include <foo.h>
    ++	# if BAR
    ++	#  include <bar.h>
    ++	# endif
     +	#endif
     +
       - We try to keep to at most 80 characters per line.
2:  7f07bf1f3b ! 3:  25f73b970d Documentation: document naming schema for struct-related functions
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    Documentation: document naming schema for struct-related functions
    +    Documentation: document naming schema for structs and their functions
     
         We nowadays have a proper mishmash of struct-related functions that are
         called `<verb>_<struct>` (e.g. `clear_prio_queue()`) versus functions
    @@ Documentation/CodingGuidelines: For C programs:
         use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb"
         ./bin-wrappers/git log` (See `wrap-for-bin.sh`.)
      
    -+ - Functions that operate on a specific structure and which are used by
    -+   other subsystems shall be named after the structure. The function
    -+   name should start with the name of the structure followed by a verb.
    -+   E.g.
    ++ - The primary data structure that a subsystem 'S' deals with is called
    ++   `struct S`. Functions that operate on `struct S` are named
    ++   `S_<verb>()` and should generally receive a pointer to `struct S` as
    ++   first parameter. E.g.
     +
     +	struct strbuf;
     +
3:  5e1de3c315 ! 4:  d4ce00303f Documentation: document difference between release and free
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    Documentation: document difference between release and free
    +    Documentation: document idiomatic function names
     
         We semi-regularly have discussions around whether a function shall be
    -    named `release()` or `free()`. For most of the part we use these two
    -    terminologies quite consistently though:
    -
    -      - `release()` only frees internal state of a structure, whereas the
    -        structure itself is not free'd.
    -
    -      - `free()` frees both internal state and the structure itself.
    +    named `S_release()`, `S_clear()` or `S_free()`. Indeed, it may not be
    +    obvious which of these is preferable as we never really defined what
    +    each of these variants means exactly.
     
         Carve out a space where we can add idiomatic names for common functions
    -    in our coding guidelines. This space can get extended in the future when
    -    we feel the need to document more idiomatic names.
    +    in our coding guidelines and define each of those functions. Like this,
    +    we can get to a shared understanding of their respective semantics and
    +    can easily point towards our style guide in future discussions such that
    +    our codebase becomes more consistent over time.
    +
    +    Note that the intent is not to rename all functions which violate these
    +    semantics right away. Rather, the intent is to slowly converge towards a
    +    common style over time.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ Documentation/CodingGuidelines: For C programs:
      	void reset_strbuf(struct strbuf *buf);
      
     + - There are several common idiomatic names for functions performing
    -+   specific tasks on structures:
    ++   specific tasks on a structure `S`:
     +
    -+    - `<struct>_init()` initializes a structure without allocating the
    ++    - `S_init()` initializes a structure without allocating the
     +      structure itself.
     +
    -+    - `<struct>_release()` releases a structure's contents without
    -+      freeing the structure.
    ++    - `S_release()` releases a structure's contents without freeing the
    ++      structure.
    ++
    ++    - `S_clear()` is equivalent to `S_release()` followed by `S_init()`
    ++      such that the structure is directly usable after clearing it. When
    ++      `S_clear()` is provided, `S_init()` shall not allocate resources
    ++      that need to be released again.
     +
    -+    - `<struct>_free()` releases a structure's contents and frees the
    ++    - `S_free()` releases a structure's contents and frees the
     +      structure.
     +
      For Perl programs:
-:  ---------- > 5:  8789323ac7 Documentation: consistently use spaces inside initializers

-- 
2.46.0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]