> Subject: [GSOC][PATCH] Fixed an issue which contained extra unnecessary code
Commit messages (and titles) should always be in the imperative mood.
The title in particular should be a short description of what the
patch is doing, and should give meaningful information to people
reading the output of 'git log --oneline'. Also we usually prefix the
title with the area of the code we're touching.
The title above is very generic, and thus not very useful to future
readers. Something like
compat/regex: remove unnecessary if
would give future readers a bit more context.
On 03/10, sushmaunnibhavi wrote:
> From 5a6c233c6bf0a35aca000b32b9e936a34532900a Mon Sep 17 00:00:00 2001
> From: sushmaunnibhavi <sushmaunnibhavi@xxxxxxxxx>
> Date: Sun, 10 Mar 2019 19:37:33 +0530
> Subject: [GSOC][PATCH] Fixed an issue which contained extra unnecessary code
The four lines above don't need to be included here, as they are
already defined by the email headers.
The one line that may be useful here is "From: sushmaunnibhavi
<sushmaunnibhavi@xxxxxxxxx>". That should be your full name, and
match the name in the Signed-off-by line. Usually people just set
that up in the "From" header in the email, but if you are unable to
configure your mailer like that, including a "From: Sushma Unnibhavi
<sushmaunnibhavi425@xxxxxxxxx>" before the rest of the commit message
also works.
> Signed-off-by: Sushma Unnibhavi <sushmaunnibhavi425@xxxxxxxxx>
> ---
> Since '\n' and '\0' are char_len==1,it is not necessary to check if the char_len<=1.
This explanation of why this is a good patch should be included in the
commit message before your Signed-off-by. If we want to apply this
change (which I don't think we want for the reasons stated below), I
think this needs a bit of a deeper analysis in the commit message, to
convince readers this is the correct thing to do.
Also please wrap commit messages at 72 characters.
> compat/regex/regexec.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/compat/regex/regexec.c b/compat/regex/regexec.c
This file in compat/ was directly imported from upstream gawk. We
generally don't patch this type of imported file, to make updates from
upstream easier, unless there is an actual fix from upstream that
needs to be fixed that's not going to be fixed upstream.
As this change at best removes a redundant 'if' (I can't comment on the
correctness, as I'm not familar with this code), so it is not worth
changing this file in our codebase.
> index 1b5d89fd5e..df62597531 100644
> --- a/compat/regex/regexec.c
> +++ b/compat/regex/regexec.c
> @@ -3799,11 +3799,6 @@ check_node_accept_bytes (const re_dfa_t *dfa, int node_idx,
> char_len = re_string_char_size_at (input, str_idx);
> if (node->type == OP_PERIOD)
> {
> - if (char_len <= 1)
> - return 0;
> - /* FIXME: I don't think this if is needed, as both '\n'
> - and '\0' are char_len == 1. */
> - /* '.' accepts any one character except the following two cases. */
> if ((!(dfa->syntax & RE_DOT_NEWLINE) &&
> re_string_byte_at (input, str_idx) == '\n') ||
> ((dfa->syntax & RE_DOT_NOT_NULL) &&
> --
> 2.17.1
>
