{"thread":{"id":"55916","subject":"[PATCH 0/2] Trivial cleanups","startedAt":"2021-06-13T06:37:07Z","lastAt":"2021-06-15T08:22:11Z","messageCount":5,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"427215","messageId":"20210613063702.269816-1-felipe.contreras@gmail.com","threadId":"55916","inReplyTo":null,"subject":"[PATCH 0/2] Trivial cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-13T06:37:00Z","receivedAt":"2021-06-13T06:37:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"These perfeclty good patches from 2014 weren't picked with no good\nreason.\n\n[1] https://lore.kernel.org/git/1398808178-3703-1-git-send-email-felipe.contreras@gmail.com/\n\nFelipe Contreras (2):\n config: avoid yoda conditions\n add: avoid yoda condition\n\n builtin/add.c | 2 +-\n config.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\n-- \n2.32.0\n\n"},{"id":"427216","messageId":"20210613063702.269816-2-felipe.contreras@gmail.com","threadId":"55916","inReplyTo":"20210613063702.269816-1-felipe.contreras@gmail.com","subject":"[PATCH 1/2] config: avoid yoda conditions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-13T06:37:01Z","receivedAt":"2021-06-13T06:37:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n config.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex f9c400ad30..7094530b04 100644\n--- a/config.c\n+++ b/config.c\n@@ -1232,7 +1232,7 @@ static int git_parse_maybe_bool_text(const char *value)\n int git_parse_maybe_bool(const char *value)\n {\n \tint v = git_parse_maybe_bool_text(value);\n-\tif (0 <= v)\n+\tif (v >= 0)\n \t\treturn v;\n \tif (git_parse_int(value, &v))\n \t\treturn !!v;\n@@ -1242,7 +1242,7 @@ int git_parse_maybe_bool(const char *value)\n int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n {\n \tint v = git_parse_maybe_bool_text(value);\n-\tif (0 <= v) {\n+\tif (v >= 0) {\n \t\t*is_bool = 1;\n \t\treturn v;\n \t}\n-- \n2.32.0\n\n"},{"id":"427217","messageId":"20210613063702.269816-3-felipe.contreras@gmail.com","threadId":"55916","inReplyTo":"20210613063702.269816-1-felipe.contreras@gmail.com","subject":"[PATCH 2/2] add: avoid yoda condition","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-13T06:37:02Z","receivedAt":"2021-06-13T06:38:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"18 is younger than person's age.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex b773b5a499..8c2a5a5094 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -510,7 +510,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \targc--;\n \targv++;\n \n-\tif (0 <= addremove_explicit)\n+\tif (addremove_explicit >= 0)\n \t\taddremove = addremove_explicit;\n \telse if (take_worktree_changes && ADDREMOVE_DEFAULT)\n \t\taddremove = 0; /* \"-u\" was given but not \"-A\" */\n-- \n2.32.0\n\n"},{"id":"427392","messageId":"xmqqbl87zyra.fsf@gitster.g","threadId":"55916","inReplyTo":"20210613063702.269816-1-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] Trivial cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-15T01:45:45Z","receivedAt":"2021-06-15T02:24:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> These perfeclty good patches from 2014 weren't picked with no good\n> reason.\n\nThese are safe no-op changes, but that does not mean they are good\npatches. It goes against Documentation/CodingGuidelines to bring it\nback again now, which is a good enough reason not to look at them.\n"},{"id":"427423","messageId":"60c8632d60027_e633208db@natae.notmuch","threadId":"55916","inReplyTo":"xmqqbl87zyra.fsf@gitster.g","subject":"Re: [PATCH 0/2] Trivial cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-15T08:22:05Z","receivedAt":"2021-06-15T08:22:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > These perfeclty good patches from 2014 weren't picked with no good\n> > reason.\n> \n> These are safe no-op changes, but that does not mean they are good\n> patches.\n\nThey are not good patches because they are no-op, they are good changes\nbecause they correct the flow of the code to match the flow in which\nmost people think.\n\n 18 is younger than Mary\n\nis not a sentence most people would agree make sense.\n\n> It goes against Documentation/CodingGuidelines to bring it\n> back again now, which is a good enough reason not to look at them.\n\nThese are not style issues. Refactoring code to make it easier to\nunderstand goes beyond style.\n\nRefactoring this:\n\n\tstatic int is_same_remote(struct remote *remote)\n\t{\n\t\tstruct remote *fetch_remote = remote_get(NULL);\n\t\treturn (!fetch_remote || fetch_remote == remote);\n\t}\n\n\tint same_remote = is_same_remote(remote);\n\nTo this:\n\n\tint same_remote = remote == remote_get(NULL);\n\nIs more than just style.\n\nBut suit yourself. Sooner or later somebody is going to fix these\nglaring mistakes. And they are mistakes [1].\n\n Yoda conditions are widely criticized for compromising readability by\n increasing the cognitive load of reading the code.\n\nCheers.\n\n[1] https://en.wikipedia.org/wiki/Yoda_conditions#Criticism\n\n-- \nFelipe Contreras\n"}]}