Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Apr 26, 2016, 16:26 UTC
- Message-ID
- <CAP8UFD2rD5qo7TBPFdP5BifOGuWHAtZA2BNGDqnY_feoCfmKeQ@mail.gmail.com>
- In-Reply-To
- <CAGZ79kYUr9wQ--898OeyqVw_upqxfVNRsOveUAPSn=PtXZ4xBQ@mail.gmail.com>
On Mon, Apr 25, 2016 at 8:57 PM, Stefan Beller <sbeller@google.com> wrote:
Show 34 quoted lines
> On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> 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.I kept the existing comments when they were still relevant. It could be a cleanup to change them to something like what you suggest, but as it is not important for this series which is already long, I prefer to leave it for now.