Re: [PATCH v2 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[Junio C Hamano <gitster@pobox.com>]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> The documentation previously omitted the UNUSED macro,
> which often led to confusion for new contributors
> when they encountered compiler warnings related to unused parameters.

The above is not quite easy to reason about.  It is more like we
wrote this document, and then later tightened the default compiler
warnings for developer builds.  So "omitted" may technically be
correct, but it was more like "did not use it, because there was no
need".

    The sample program, as written, would not build for at least two
    reasons:

    - Since this document was first written, the calling convention
      to subcommand implementation has changed, and now cmd_psuh()
      needs to accept the third parameter, repository.

    - These days, compiler warning options for developers include
      one that detects and complains about unused parameters, so
      ones that are deliberately unused have to be marked as such.

After such observation on the status quo and description of the
problem you are going to solve, you give an order to the code base
to fix it, perhaps like:

    Update the old-style examples to adjust to the current
    practices, with explanations as needed.


To recap, the usual way to compose a log message of this project is
to

 - Give an observation on how the current system works in the
   present tense (so no need to say "Currently X is Y", or
   "Previously X was Y" to describe the state before your change;
   just "X is Y" is enough), and discuss what you perceive as a
   problem in it.

 - Propose a solution (optional---often, problem description
   trivially leads to an obvious solution in reader's minds).

 - Give commands to the codebase to "become like so".

in this order.

>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ef190d8748..f4320d8869 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -142,7 +142,15 @@ command in `builtin/psuh.c`. Create that file, and within it, write the entry
>  point for your command in a function matching the style and signature:
>  
>  ----
> -int cmd_psuh(int argc, const char **argv, const char *prefix)
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
> +----
> +
> +We will use the UNUSED macro to make sure we don't recieve compiler warnings
> +for unused arguments from the function cmd_psuh.
> +----
> +int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
> +	    const char *prefix UNUSED, struct repository *repo UNUSED)
>  ----

I do not quite understand.  Why do we need a new one here?  Wouldn't
it be easier to read for a newcomer if you just give the last one
and explain what UNUSED are for?  Perhaps like

    ... matching the style and signature:

    ----
    int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
	         const char *prefix UNUSED, struct repository *repo UNUSED)
    ----

    A few things to note:

    * A subcommand implementation takes its command line arguments
      in `int argc` + `const char **argv`, like `main()` would

    * It also takes two extra parameters, `prefix` and `repo`.  What
      they mean will not be discussed until much later.

    * Because this first example will not use any of the parameters,
      your compiler will give warnings on unused parameters.  As the
      list of these four parameters is mandated by the API to add
      new built-in commands, you cannot omit them.  Instead, you add
      `UNUSED` to each of them to tell the compiler that you _know_
      you are not (yet) using it.

Take a special note on the last one.  There may be multiple ways to
squelch warnings, but it is worth telling your readers that use of
UNUSED is the right way.

I'll stop here for this patch.

Thanks.