Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'

From: Stefan Beller <sbeller@google.com>
Date: Apr 25, 2016, 18:57 UTC
Message-ID: <CAGZ79kYUr9wQ--898OeyqVw_upqxfVNRsOveUAPSn=PtXZ4xBQ@mail.gmail.com>
In-Reply-To: <1461504863-15946-10-git-send-email-chriscool@tuxfamily.org>

On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 16 quoted lines

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/apply.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index ad81210..6c628f6 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -25,12 +25,15 @@ struct apply_state {
>         const char *prefix;
>         int prefix_length;
>
> +       /*
> +        *  --check turns on checking that the working tree matches the
> +        *    files that are being modified, but doesn't apply the patch

This is true, but at this part of the file/code we rather want to know what `check` does, instead of what the command line option --check does. (They are 2 different things, though one leading to the other one?) How about:

    /*
     * Only check the files to be modified, but do not modify the files.
     */

>  /*
> - *  --check turns on checking that the working tree matches the
> - *    files that are being modified, but doesn't apply the patch

Oh I see it was moved from here. Not sure if we want to rename comments along the way or just keep it in this series.