Felipe ContrerasThese perfeclty good patches from 2014 weren't picked with no good reason.
[1] https://lore.kernel.org/git/1398808178-3703-1-git-send-email-felipe.contreras@gmail.com/
Felipe Contreras (2): config: avoid yoda conditions add: avoid yoda condition
builtin/add.c | 2 +- config.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
-- 2.32.0
Felipe Contreras[PATCH 1/2] config: avoid yoda conditions
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c index f9c400ad30..7094530b04 100644 --- a/config.c +++ b/config.c @@ -1232,7 +1232,7 @@ static int git_parse_maybe_bool_text(const char *value) int git_parse_maybe_bool(const char *value) { int v = git_parse_maybe_bool_text(value); - if (0 <= v) + if (v >= 0) return v; if (git_parse_int(value, &v)) return !!v; @@ -1242,7 +1242,7 @@ int git_parse_maybe_bool(const char *value) int git_config_bool_or_int(const char *name, const char *value, int *is_bool) { int v = git_parse_maybe_bool_text(value); - if (0 <= v) { + if (v >= 0) { *is_bool = 1; return v; }
-- 2.32.0
Felipe Contreras[PATCH 2/2] add: avoid yoda condition
18 is younger than person's age.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/add.c b/builtin/add.c index b773b5a499..8c2a5a5094 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -510,7 +510,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) argc--; argv++; - if (0 <= addremove_explicit) + if (addremove_explicit >= 0) addremove = addremove_explicit; else if (take_worktree_changes && ADDREMOVE_DEFAULT) addremove = 0; /* "-u" was given but not "-A" */
-- 2.32.0
Junio C HamanoRe: [PATCH 0/2] Trivial cleanups
Felipe Contreras <felipe.contreras@gmail.com> writes:
> These perfeclty good patches from 2014 weren't picked with no good > reason.
These are safe no-op changes, but that does not mean they are good patches. It goes against Documentation/CodingGuidelines to bring it back again now, which is a good enough reason not to look at them.
Felipe ContrerasRe: [PATCH 0/2] Trivial cleanups
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes: > > > These perfeclty good patches from 2014 weren't picked with no good > > reason. > > These are safe no-op changes, but that does not mean they are good > patches.
They are not good patches because they are no-op, they are good changes because they correct the flow of the code to match the flow in which most people think.
18 is younger than Mary
is not a sentence most people would agree make sense.
> It goes against Documentation/CodingGuidelines to bring it > back again now, which is a good enough reason not to look at them.
These are not style issues. Refactoring code to make it easier to understand goes beyond style.
Refactoring this:
static int is_same_remote(struct remote *remote)
{
struct remote *fetch_remote = remote_get(NULL);
return (!fetch_remote || fetch_remote == remote);
}int same_remote = is_same_remote(remote);
To this:
int same_remote = remote == remote_get(NULL);
Is more than just style.
But suit yourself. Sooner or later somebody is going to fix these glaring mistakes. And they are mistakes [1].
Yoda conditions are widely criticized for compromising readability by increasing the cognitive load of reading the code.
Cheers.
[1] https://en.wikipedia.org/wiki/Yoda_conditions#Criticism
-- Felipe Contreras