On Thu, Dec 29, 2022 at 12:18 PM Rose via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> Signed-off-by: Seija Kijin <doremylover123@xxxxxxxxx>
> ---
> diff --git a/grep.c b/grep.c
> @@ -1483,9 +1483,12 @@ static int fill_textconv_grep(struct repository *r,
> static int is_empty_line(const char *bol, const char *eol)
> {
> - while (bol < eol && isspace(*bol))
> + while (bol < eol) {
> + if (!isspace(*bol))
> + return 0;
> bol++;
> - return bol == eol;
> + }
> + return 1;
> }
It is subjective (personal opinion) whether or not the new code is
clearer than the original. As a general policy, this project tends not
to accept patches like this which merely "churn" the code without
improving it. From Documentation/SubmittingPatches:
"Once it _is_ in the tree, it's not really worth the patch noise to
go and fix it up."
Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html
One reason for avoiding churn is that even simple and innocuous
changes like this can introduce bugs or unwanted behavior, as v1 of
this patch illustrated[1]. Another reason is that it eats up reviewer
time.
Did the Git test suite pass with v1 of this patch even though it was
buggy? If so, a better use of your time and reviewer time would be to
improve test coverage so that it detects the sort of breakage caused
by v1.
[1]: https://lore.kernel.org/git/CAPig+cSFAUcU74qULYkN7OX4-OqU_3VJeTdb1ZH_QoOW9FBwZQ@xxxxxxxxxxxxxx/
