{"thread":{"id":"36106","subject":"[PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","startedAt":"2014-03-10T07:39:38Z","lastAt":"2014-03-11T08:11:34Z","messageCount":14,"participants":["Nemina Amarasinghe","Matthieu Moy","David Kastrup","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236351","messageId":"loom.20140310T083649-236@post.gmane.org","threadId":"36106","inReplyTo":null,"subject":"[PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-10T07:39:38Z","receivedAt":"2014-03-10T07:39:38Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"Did some simple changing to the chain of if() statements in function\ninstall_branch_config() of branch.c\nThis was a micro-project for GSOC 2014 \n\n\n>From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001\nFrom: Nemina Amarasinghe <neminaa@gmail.com>\nDate: Mon, 10 Mar 2014 13:02:55 +0530\nSubject: [PATCH] simplified the chain if() statements of\n install_branch_config() function in branch.c\n\n---\n branch.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/branch.c b/branch.c\nindex d3b9d49..0304a7a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -87,7 +87,12 @@ void install_branch_config(int flag, const char *local,\nconst char *origin, cons\n \t\t\t\t  _(\"Branch %s set up to track local branch %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track local branch %s.\"),\n \t\t\t\t  local, shortname);\n-\t\telse if (!remote_is_branch && (origin || !origin))\n+\t\telse if (!remote_is_branch && origin)\n+\t\t\tprintf_ln(rebasing ?\n+\t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n+\t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n+\t\t\t\t  local, remote);\n+\t\telse if (!remote_is_branch && !origin)\n \t\t\tprintf_ln(rebasing ?\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n-- \n1.9.0.152.g6ab4ae2\n"},{"id":"236353","messageId":"loom.20140310T085652-521@post.gmane.org","threadId":"36106","inReplyTo":"loom.20140310T083649-236@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-10T07:58:45Z","receivedAt":"2014-03-10T07:58:45Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n\nSorry for the first patch. Something went wrong. This is the correct one\n\n\n>From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001\nFrom: Nemina Amarasinghe <neminaa@gmail.com>\nDate: Mon, 10 Mar 2014 13:02:55 +0530\nSubject: [PATCH] simplified the chain if() statements of\n install_brach_config() function in branch.c\n\n---\n branch.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/branch.c b/branch.c\nindex d3b9d49..0304a7a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local,\nconst char *origin, cons\n \t\t\t\t  _(\"Branch %s set up to track local branch %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track local branch %s.\"),\n \t\t\t\t  local, shortname);\n-\t\telse if (!remote_is_branch && origin)\n-\t\t\tprintf_ln(rebasing ?\n-\t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n-\t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n-\t\t\t\t  local, remote);\n-\t\telse if (!remote_is_branch && !origin)\n+\t\telse if (!remote_is_branch && (origin || !origin))\n \t\t\tprintf_ln(rebasing ?\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n-- \n1.9.0.152.g6ab4ae2\n"},{"id":"236355","messageId":"vpqd2huihsc.fsf@anie.imag.fr","threadId":"36106","inReplyTo":"loom.20140310T085652-521@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-03-10T08:23:15Z","receivedAt":"2014-03-10T08:23:15Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nemina Amarasinghe <neminaa@gmail.com> writes:\n\n> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>\n> Sorry for the first patch. Something went wrong. This is the correct one\n\nPlease, re-read Documentation/SubmittingPatches. In short, don't inline\npatch headers and don't forget the sign-off.\n\n> Subject: [PATCH] simplified the chain if() statements of\n>  install_brach_config() function in branch.c\n\nKeep the subject line short (ideally <50 characters), and avoid past\ntense. We usually use imperative (the patch asks the codebase to\nrefactor itself => \"simplify if statements ...\". We usually prefix the\nsubject line with the place/subsystem where the change is done =>\n\"branch.c: simplify if ...\".\n\n> -\t\telse if (!remote_is_branch && origin)\n> -\t\t\tprintf_ln(rebasing ?\n> -\t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n> -\t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n> -\t\t\t\t  local, remote);\n> -\t\telse if (!remote_is_branch && !origin)\n> +\t\telse if (!remote_is_branch && (origin || !origin))\n\nIs it me, or is (origin || !origin) a tautology?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"236358","messageId":"87d2hua1rk.fsf@fencepost.gnu.org","threadId":"36106","inReplyTo":"vpqd2huihsc.fsf@anie.imag.fr","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-10T08:36:31Z","receivedAt":"2014-03-10T08:36:31Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Nemina Amarasinghe <neminaa@gmail.com> writes:\n>\n>> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>>\n>> Sorry for the first patch. Something went wrong. This is the correct one\n>\n> Please, re-read Documentation/SubmittingPatches. In short, don't inline\n> patch headers and don't forget the sign-off.\n>\n>> Subject: [PATCH] simplified the chain if() statements of\n>>  install_brach_config() function in branch.c\n>\n> Keep the subject line short (ideally <50 characters), and avoid past\n> tense. We usually use imperative (the patch asks the codebase to\n> refactor itself => \"simplify if statements ...\". We usually prefix the\n> subject line with the place/subsystem where the change is done =>\n> \"branch.c: simplify if ...\".\n>\n>> -\t\telse if (!remote_is_branch && origin)\n>> -\t\t\tprintf_ln(rebasing ?\n>> - _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>> -\t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n>> -\t\t\t\t  local, remote);\n>> -\t\telse if (!remote_is_branch && !origin)\n>> +\t\telse if (!remote_is_branch && (origin || !origin))\n>\n> Is it me, or is (origin || !origin) a tautology?\n\nSince it _is_ you, that question is a tautology because of its first\npart already.  But the second one would be just as good.\n\n-- \nDavid Kastrup\n"},{"id":"236360","messageId":"loom.20140310T094407-5@post.gmane.org","threadId":"36106","inReplyTo":"vpqd2huihsc.fsf@anie.imag.fr","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-10T08:51:05Z","receivedAt":"2014-03-10T08:51:05Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"> \n> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n> \n> Is it me, or is (origin || !origin) a tautology?\n> \nThanks for the advices Matthieu. I will go through the documentations again. \nIs there anything wrong with my logic? \nWhat I wanted to express is\n((!remote_is_branch && origin) || (!remote_is_branch || !origin))\n"},{"id":"236361","messageId":"vpqha76e842.fsf@anie.imag.fr","threadId":"36106","inReplyTo":"loom.20140310T094407-5@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-03-10T09:05:49Z","receivedAt":"2014-03-10T09:05:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nemina Amarasinghe <neminaa@gmail.com> writes:\n\n>> \n>> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>> \n>> Is it me, or is (origin || !origin) a tautology?\n>> \n> Thanks for the advices Matthieu. I will go through the documentations again. \n> Is there anything wrong with my logic? \n> What I wanted to express is\n> ((!remote_is_branch && origin) || (!remote_is_branch || !origin))\n\n(The second || should be a && in this sentence)\n\nI'm not saying your rewrite is incorrect, but there's no reason to keep\n(origin || !origin) in a logical formula.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"236363","messageId":"878usia09y.fsf@fencepost.gnu.org","threadId":"36106","inReplyTo":"loom.20140310T094407-5@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-10T09:08:41Z","receivedAt":"2014-03-10T09:08:41Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Nemina Amarasinghe <neminaa@gmail.com> writes:\n\n>> \n>> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>> \n>> Is it me, or is (origin || !origin) a tautology?\n>> \n> Thanks for the advices Matthieu. I will go through the documentations again. \n> Is there anything wrong with my logic? \n> What I wanted to express is\n> ((!remote_is_branch && origin) || (!remote_is_branch || !origin))\n\nIs it?\n\nThe above is the same as (!remote_is_branch || !origin).  What you wrote\nbefore is the same as (!remote_is_branch).\n\nMaybe you should try copy&paste from the expressions you are trying to\ncombine to make sure that what you start with makes sense.\n\n-- \nDavid Kastrup\n"},{"id":"236364","messageId":"loom.20140310T101112-236@post.gmane.org","threadId":"36106","inReplyTo":"878usia09y.fsf@fencepost.gnu.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-10T09:15:10Z","receivedAt":"2014-03-10T09:15:10Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"\n> > ((!remote_is_branch && origin) || (!remote_is_branch || !origin))\n> \n> Is it?\n> \n> The above is the same as (!remote_is_branch || !origin).  What you wrote\n> before is the same as (!remote_is_branch).\n> \n> Maybe you should try copy&paste from the expressions you are trying to\n> combine to make sure that what you start with makes sense.\n> \nOMG.. Really sorry for that... that was a silly mistake. \nThis is the one..\n\n((!remote_is_branch && origin) || (!remote_is_branch && !origin))\n"},{"id":"236365","messageId":"874n369zet.fsf@fencepost.gnu.org","threadId":"36106","inReplyTo":"loom.20140310T101112-236@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-10T09:27:22Z","receivedAt":"2014-03-10T09:27:22Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Nemina Amarasinghe <neminaa@gmail.com> writes:\n\n>> > ((!remote_is_branch && origin) || (!remote_is_branch || !origin))\n>> \n>> Is it?\n>> \n>> The above is the same as (!remote_is_branch || !origin).  What you wrote\n>> before is the same as (!remote_is_branch).\n>> \n>> Maybe you should try copy&paste from the expressions you are trying to\n>> combine to make sure that what you start with makes sense.\n>> \n> OMG.. Really sorry for that... that was a silly mistake. \n> This is the one..\n>\n> ((!remote_is_branch && origin) || (!remote_is_branch && !origin))\n\nThat is, indeed, perfectly equivalent to (!remote_is_branch).  If you\nwrite\n\n(!remote_is_branch && (origin || !origin))\n\nthen you will have people (and possibly also the compiler) loudly\nwondering about what you are trying to say here.  The suspicion would be\nthat either this is a result of a typo or is supposed to be an\nannoyingly obtuse replacement for a\n/* TODO: treat origin and !origin differently */\nkind of comment.\n\n-- \nDavid Kastrup\n"},{"id":"236458","messageId":"CAPig+cRAqEQ8320Gq2Njoioq5UU=d+W+hzF+YZAZ7Po9XjMq0g@mail.gmail.com","threadId":"36106","inReplyTo":"loom.20140310T085652-521@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-10T20:46:13Z","receivedAt":"2014-03-10T20:46:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe <neminaa@gmail.com> wrote:\n> Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>\n> Sorry for the first patch. Something went wrong. This is the correct one\n\nIn addition to the tautological issue pointed out by Matthieu, please\nnote that this version of the patch is not the correct one. It won't\napply to the git code. At a guess, it appears that this patch is\nagainst some other modification you made to the git code before this\nchange, or perhaps you committed it incorrectly. In any event, when\nyou resubmit, please be sure that the new version can be applied to\ncommit.c as it exists in git.git itself.\n\n>\n> From aebfa60feb643280c89b54e5ab87f9d960cde452 Mon Sep 17 00:00:00 2001\n> From: Nemina Amarasinghe <neminaa@gmail.com>\n> Date: Mon, 10 Mar 2014 13:02:55 +0530\n> Subject: [PATCH] simplified the chain if() statements of\n>  install_brach_config() function in branch.c\n>\n> ---\n>  branch.c | 7 ++++++-\n>  1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/branch.c b/branch.c\n> index d3b9d49..0304a7a 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local,\n> const char *origin, cons\n>                                   _(\"Branch %s set up to track local branch %s by rebasing.\") :\n>                                   _(\"Branch %s set up to track local branch %s.\"),\n>                                   local, shortname);\n> -               else if (!remote_is_branch && origin)\n> -                       printf_ln(rebasing ?\n> -                                 _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n> -                                 _(\"Branch %s set up to track remote ref %s.\"),\n> -                                 local, remote);\n> -               else if (!remote_is_branch && !origin)\n> +               else if (!remote_is_branch && (origin || !origin))\n>                         printf_ln(rebasing ?\n>                                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                                   _(\"Branch %s set up to track remote ref %s.\"),\n> --\n> 1.9.0.152.g6ab4ae2\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"236483","messageId":"loom.20140311T072612-596@post.gmane.org","threadId":"36106","inReplyTo":"CAPig+cRAqEQ8320Gq2Njoioq5UU=d+W+hzF+YZAZ7Po9XjMq0g@mail.gmail.com","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-11T06:30:04Z","receivedAt":"2014-03-11T06:30:04Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"Eric Sunshine <sunshine <at> sunshineco.com> writes:\n\n> \n> On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe <neminaa <at>\ngmail.com> wrote:\n> > Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n> >\n> > Sorry for the first patch. Something went wrong. This is the correct one\n> \n> In addition to the tautological issue pointed out by Matthieu, please\n> note that this version of the patch is not the correct one. It won't\n> apply to the git code. At a guess, it appears that this patch is\n> against some other modification you made to the git code before this\n> change, or perhaps you committed it incorrectly. In any event, when\n> you resubmit, please be sure that the new version can be applied to\n> commit.c as it exists in git.git itself.\n> \n\nThank you very much for your comments Eric. I will resubmit the patch.\n\nJust a quick question\n\nin this code\n\nif (flag & BRANCH_CONFIG_VERBOSE) {\n        if (remote_is_branch && origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote branch %s from %s by\nrebasing.\") :\n                  _(\"Branch %s set up to track remote branch %s from %s.\"),\n                  local, shortname, origin);\n        else if (remote_is_branch && !origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track local branch %s by rebasing.\") :\n                  _(\"Branch %s set up to track local branch %s.\"),\n                  local, shortname);\n        else if (!remote_is_branch && origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n                  _(\"Branch %s set up to track remote ref %s.\"),\n                  local, remote);\n        else if (!remote_is_branch && !origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n                  _(\"Branch %s set up to track remote ref %s.\"),\n                  local, remote);\n        else\n            die(\"BUG: impossible combination of %d and %p\",\n                remote_is_branch, origin);\n    }\n\nThese \"local\" and \"remote\" variables are independent from the \"origin\" right?\nSo, If that the case couldn't we just use the bellow function\n\n\nelse if (!remote_is_branch)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n                  _(\"Branch %s set up to track remote ref %s.\"),\n                  local, remote);\n\ninstead of\n\nelse if (!remote_is_branch && origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n                  _(\"Branch %s set up to track remote ref %s.\"),\n                  local, remote);\n        else if (!remote_is_branch && !origin)\n            printf_ln(rebasing ?\n                  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n                  _(\"Branch %s set up to track remote ref %s.\"),\n                  local, remote);\n\nThanks\nNemina  \n"},{"id":"236484","messageId":"loom.20140311T081018-894@post.gmane.org","threadId":"36106","inReplyTo":"CAPig+cRAqEQ8320Gq2Njoioq5UU=d+W+hzF+YZAZ7Po9XjMq0g@mail.gmail.com","subject":"Re: [PATCH][GSoC]simplified branch.c:install_branch_config() if() statement","fromName":"Nemina Amarasinghe","fromEmail":"neminaa@gmail.com","sentAt":"2014-03-11T07:16:22Z","receivedAt":"2014-03-11T07:16:22Z","isPatch":true,"sender":{"key":"neminaa@gmail.com","avatar":null},"body":"I hope this is the correct format for  patch. Please comment on this if\nsomething is wrong. \n\nSigned-off-by:Nemina Amarasinghe <neminaa@gmail.com>\n---\n branch.c | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 0304a7a..fd93603 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local,\nconst char *origin, cons\n \t\t\t\t  _(\"Branch %s set up to track local branch %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track local branch %s.\"),\n \t\t\t\t  local, shortname);\n-\t\telse if (!remote_is_branch && origin)\n-\t\t\tprintf_ln(rebasing ?\n-\t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n-\t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n-\t\t\t\t  local, remote);\n-\t\telse if (!remote_is_branch && !origin)\n+\t\telse if (!remote_is_branch)\n \t\t\tprintf_ln(rebasing ?\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n \t\t\t\t  _(\"Branch %s set up to track remote ref %s.\"),\n-- \n1.9.0.152.g6ab4ae2\n"},{"id":"236486","messageId":"CAPig+cSfZZUNfcqJTEabqw_SVJwx0qgDK+qbpYZrEoPvJwT-ig@mail.gmail.com","threadId":"36106","inReplyTo":"loom.20140311T072612-596@post.gmane.org","subject":"Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-11T08:06:15Z","receivedAt":"2014-03-11T08:06:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 11, 2014 at 2:30 AM, Nemina Amarasinghe <neminaa@gmail.com> wrote:\n> Eric Sunshine <sunshine <at> sunshineco.com> writes:\n>>\n>> On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe <neminaa <at>\n> gmail.com> wrote:\n>> > Nemina Amarasinghe <neminaa <at> gmail.com> writes:\n>> >\n>> > Sorry for the first patch. Something went wrong. This is the correct one\n>>\n>> In addition to the tautological issue pointed out by Matthieu, please\n>> note that this version of the patch is not the correct one. It won't\n>> apply to the git code. At a guess, it appears that this patch is\n>> against some other modification you made to the git code before this\n>> change, or perhaps you committed it incorrectly. In any event, when\n>> you resubmit, please be sure that the new version can be applied to\n>> commit.c as it exists in git.git itself.\n>>\n>\n> Thank you very much for your comments Eric. I will resubmit the patch.\n>\n> Just a quick question\n>\n> in this code\n>\n> if (flag & BRANCH_CONFIG_VERBOSE) {\n>         if (remote_is_branch && origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote branch %s from %s by\n> rebasing.\") :\n>                   _(\"Branch %s set up to track remote branch %s from %s.\"),\n>                   local, shortname, origin);\n>         else if (remote_is_branch && !origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track local branch %s by rebasing.\") :\n>                   _(\"Branch %s set up to track local branch %s.\"),\n>                   local, shortname);\n>         else if (!remote_is_branch && origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                   _(\"Branch %s set up to track remote ref %s.\"),\n>                   local, remote);\n>         else if (!remote_is_branch && !origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                   _(\"Branch %s set up to track remote ref %s.\"),\n>                   local, remote);\n>         else\n>             die(\"BUG: impossible combination of %d and %p\",\n>                 remote_is_branch, origin);\n>     }\n\nThis code does not exist in git.git. Somehow, the code you are\nconsulting has incorrectly substituted 'remote' for 'local' in the\nstrings of the (!remote_is_branch && !origin) case. Whether this is\ndue to a local modification you made or some some other reason, it is\nleading you astray in your proposed changes and breaking your patches\nso that they cannot be applied to git.git.\n\nProbably easiest at this point would be for you to start from scratch\nby making a new clone of git.git and examining branch.c as it actually\nexists in the 'master' branch.\n\n> These \"local\" and \"remote\" variables are independent from the \"origin\" right?\n> So, If that the case couldn't we just use the bellow function\n>\n>\n> else if (!remote_is_branch)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                   _(\"Branch %s set up to track remote ref %s.\"),\n>                   local, remote);\n>\n> instead of\n>\n> else if (!remote_is_branch && origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                   _(\"Branch %s set up to track remote ref %s.\"),\n>                   local, remote);\n>         else if (!remote_is_branch && !origin)\n>             printf_ln(rebasing ?\n>                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                   _(\"Branch %s set up to track remote ref %s.\"),\n>                   local, remote);\n>\n> Thanks\n> Nemina\n>\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"236487","messageId":"CAPig+cT2You1WLkyoWbwo84xMQGyyeu6t0HB7tcsnyN+HxiLLQ@mail.gmail.com","threadId":"36106","inReplyTo":"loom.20140311T081018-894@post.gmane.org","subject":"Re: [PATCH][GSoC]simplified branch.c:install_branch_config() if() statement","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-11T08:11:34Z","receivedAt":"2014-03-11T08:11:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 11, 2014 at 3:16 AM, Nemina Amarasinghe <neminaa@gmail.com> wrote:\n> Subject: simplified branch.c:install_branch_config() if() statement\n\nUse imperative tone: \"simplify ...\"\n\n> I hope this is the correct format for  patch. Please comment on this if\n> something is wrong.\n\nThis commentary is relevant to the email discussion but not to the\ncommit description, so it should be placed below the \"---\" line just\nunder your sign-0ff.\n\n> Signed-off-by:Nemina Amarasinghe <neminaa@gmail.com>\n> ---\n>  branch.c | 7 +------\n>  1 file changed, 1 insertion(+), 6 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index 0304a7a..fd93603 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -87,12 +87,7 @@ void install_branch_config(int flag, const char *local,\n> const char *origin, cons\n>                                   _(\"Branch %s set up to track local branch %s by rebasing.\") :\n>                                   _(\"Branch %s set up to track local branch %s.\"),\n>                                   local, shortname);\n> -               else if (!remote_is_branch && origin)\n> -                       printf_ln(rebasing ?\n> -                                 _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n> -                                 _(\"Branch %s set up to track remote ref %s.\"),\n> -                                 local, remote);\n> -               else if (!remote_is_branch && !origin)\n> +               else if (!remote_is_branch)\n>                         printf_ln(rebasing ?\n>                                   _(\"Branch %s set up to track remote ref %s by rebasing.\") :\n>                                   _(\"Branch %s set up to track remote ref %s.\"),\n\nThe patch itself is broken in a couple ways.\n\nFirst, it is whitespace-damaged, possibly due to being pasted into\nyour email client. Using \"git send-email\" can help avoid this problem.\n\nSecond, the code against which this patch was made does not exist in\ngit.git. You are likely making this change atop some other local\nmodifications which you already made. Simplest at this point would\nprobably be for you to make a fresh clone of git.git and start from\nscratch by editing branch.c in the 'master' branch.\n\n> --\n> 1.9.0.152.g6ab4ae2\n"}]}