{"thread":{"id":"55794","subject":"[PATCH 00/11] Unconvolutize push.default=simple","startedAt":"2021-05-28T20:10:19Z","lastAt":"2021-05-29T05:38:49Z","messageCount":24,"participants":["Felipe Contreras","Elijah Newren","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"425822","messageId":"20210528201014.2175179-1-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":null,"subject":"[PATCH 00/11] Unconvolutize push.default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:03Z","receivedAt":"2021-05-28T20:10:19Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Tired of jumping through hoops trying to understand what the \"simple\"\nmode does, I decided to reorganize it up for good so it's crystal\nclear.\n\nThere are no functional changes.\n\nBasically the simple mode pushes the current branch with the same name\non the remote.\n\nExcept... when there's no upstream branch configured with the same name.\n\nNow the code and the documentation are clear.\n\nThis has the additional advantage of making `current` based on `simple`,\nrather than the other way around; `current` is basically `simple`\nbut assuming we are never in a centralized workflow.\n\nFelipe Contreras (11):\n push: hedge code of default=simple\n push: move code to setup_push_simple()\n push: reorganize setup_push_simple()\n push: simplify setup_push_simple()\n push: remove unused code in setup_push_upstream()\n push: merge current and simple\n push: remove redundant check\n push: fix Yoda condition\n push: remove trivial function\n push: flip !triangular for centralized\n doc: push: explain default=simple correctly\n\n Documentation/config/push.txt | 13 +++++-----\n builtin/push.c | 47 +++++++++++++++++++----------------\n 2 files changed, 31 insertions(+), 29 deletions(-)\n\n-- \n2.32.0.rc0\n\n"},{"id":"425823","messageId":"20210528201014.2175179-2-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 01/11] push: hedge code of default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:04Z","receivedAt":"2021-05-28T20:10:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"`simple` is the most important mode so move the relevant code to its own\nfunction to make it easier to see what it's doing.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 194967ed79..7045e4ef0c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -223,6 +223,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n+static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n+{\n+\tif (triangular)\n+\t\tsetup_push_current(remote, branch);\n+\telse\n+\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+}\n+\n static int is_workflow_triangular(struct remote *remote)\n {\n \tstruct remote *fetch_remote = remote_get(NULL);\n@@ -242,10 +250,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tif (triangular)\n-\t\t\tsetup_push_current(remote, branch);\n-\t\telse\n-\t\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+\t\tsetup_push_simple(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-- \n2.32.0.rc0\n\n"},{"id":"425824","messageId":"20210528201014.2175179-3-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 02/11] push: move code to setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:05Z","receivedAt":"2021-05-28T20:10:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In order to avoid doing unnecessary things and simplify it in further\npatches.\n\nThe code is copied exactly as-is; no functional changes.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 36 ++++++++++++++++++++++++++++++++----\n 1 file changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 7045e4ef0c..d173c39283 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,10 +225,38 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tif (triangular)\n-\t\tsetup_push_current(remote, branch);\n-\telse\n-\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+\tif (triangular) {\n+\t\tif (!branch)\n+\t\t\tdie(_(message_detached_head_die), remote->name);\n+\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\t} else {\n+\t\tif (!branch)\n+\t\t\tdie(_(message_detached_head_die), remote->name);\n+\t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n+\t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n+\t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \" git push --set-upstream %s %s\\n\"),\n+\t\t\t branch->name,\n+\t\t\t remote->name,\n+\t\t\t branch->name);\n+\t\tif (branch->merge_nr != 1)\n+\t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n+\t\t\t \"refusing to push.\"), branch->name);\n+\t\tif (triangular)\n+\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n+\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n+\t\t\t \"to update which remote branch.\"),\n+\t\t\t remote->name, branch->name);\n+\n+\t\tif (1) {\n+\t\t\t/* Additional safety */\n+\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\t\t\tdie_push_simple(branch, remote);\n+\t\t}\n+\n+\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n+\t}\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425825","messageId":"20210528201014.2175179-4-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 03/11] push: reorganize setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:06Z","receivedAt":"2021-05-28T20:10:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Simply move the code around.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex d173c39283..9c807ed707 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,13 +225,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n+\tconst char *dst;\n+\n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\n \tif (triangular) {\n-\t\tif (!branch)\n-\t\t\tdie(_(message_detached_head_die), remote->name);\n-\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\t\tdst = branch->refname;\n \t} else {\n-\t\tif (!branch)\n-\t\t\tdie(_(message_detached_head_die), remote->name);\n \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n@@ -243,20 +244,14 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\tif (branch->merge_nr != 1)\n \t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n \t\t\t \"refusing to push.\"), branch->name);\n-\t\tif (triangular)\n-\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n-\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n-\t\t\t \"to update which remote branch.\"),\n-\t\t\t remote->name, branch->name);\n-\n-\t\tif (1) {\n-\t\t\t/* Additional safety */\n-\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n-\t\t\t\tdie_push_simple(branch, remote);\n-\t\t}\n \n-\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n+\t\t/* Additional safety */\n+\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\t\tdie_push_simple(branch, remote);\n+\n+\t\tdst = branch->merge[0]->src;\n \t}\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425826","messageId":"20210528201014.2175179-5-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 04/11] push: simplify setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:07Z","receivedAt":"2021-05-28T20:10:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"branch->refname can never be different from branch->merge[0]->src.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 9c807ed707..73fe083682 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,14 +225,10 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tconst char *dst;\n-\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n-\tif (triangular) {\n-\t\tdst = branch->refname;\n-\t} else {\n+\tif (!triangular) {\n \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n@@ -248,10 +244,8 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\t/* Additional safety */\n \t\tif (strcmp(branch->refname, branch->merge[0]->src))\n \t\t\tdie_push_simple(branch, remote);\n-\n-\t\tdst = branch->merge[0]->src;\n \t}\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425827","messageId":"20210528201014.2175179-6-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 05/11] push: remove unused code in setup_push_upstream()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:08Z","receivedAt":"2021-05-28T20:10:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now it's not used for the simple mode.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 73fe083682..0961826269 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -186,7 +186,7 @@ static const char message_detached_head_die[] =\n \t \" git push %s HEAD:\\n\");\n \n static void setup_push_upstream(struct remote *remote, struct branch *branch,\n-\t\t\t\tint triangular, int simple)\n+\t\t\t\tint triangular)\n {\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n@@ -207,12 +207,6 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \t\t \"to update which remote branch.\"),\n \t\t remote->name, branch->name);\n \n-\tif (simple) {\n-\t\t/* Additional safety */\n-\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n-\t\t\tdie_push_simple(branch, remote);\n-\t}\n-\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n }\n \n@@ -271,7 +265,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote, branch, triangular, 0);\n+\t\tsetup_push_upstream(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-- \n2.32.0.rc0\n\n"},{"id":"425828","messageId":"20210528201014.2175179-7-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 06/11] push: merge current and simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:09Z","receivedAt":"2021-05-28T20:10:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"`current` is basically the same as `simple` except always assuming we\nare in a triangular workflow.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 9 +--------\n 1 file changed, 1 insertion(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 0961826269..2d6358f776 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -210,13 +210,6 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n }\n \n-static void setup_push_current(struct remote *remote, struct branch *branch)\n-{\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n-}\n-\n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n \tif (!branch)\n@@ -269,7 +262,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tsetup_push_current(remote, branch);\n+\t\tsetup_push_simple(remote, branch, 1);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_NOTHING:\n-- \n2.32.0.rc0\n\n"},{"id":"425829","messageId":"20210528201014.2175179-8-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 07/11] push: remove redundant check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:10Z","receivedAt":"2021-05-28T20:10:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If fetch_remote is NULL (i.e. the branch remote is invalid), then it\ncan't possibly be same as remote, which can't be NULL.\n\nThe check is redundant, and so is the extra variable.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2d6358f776..f008cd624f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -237,8 +237,7 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \n static int is_workflow_triangular(struct remote *remote)\n {\n-\tstruct remote *fetch_remote = remote_get(NULL);\n-\treturn (fetch_remote && fetch_remote != remote);\n+\treturn remote_get(NULL) != remote;\n }\n \n static void setup_default_push_refspecs(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425830","messageId":"20210528201014.2175179-9-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 08/11] push: fix Yoda condition","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:11Z","receivedAt":"2021-05-28T20:10:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want to check if remote is the same as the branch remote, not the\nother way around.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f008cd624f..2dda1724cc 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -237,7 +237,7 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \n static int is_workflow_triangular(struct remote *remote)\n {\n-\treturn remote_get(NULL) != remote;\n+\treturn remote != remote_get(NULL);\n }\n \n static void setup_default_push_refspecs(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425831","messageId":"20210528201014.2175179-10-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 09/11] push: remove trivial function","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:12Z","receivedAt":"2021-05-28T20:10:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's a single line that is used in a single place, and the variable has\nthe same name as the function.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2dda1724cc..8ecfbe8d63 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -235,15 +235,10 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n-static int is_workflow_triangular(struct remote *remote)\n-{\n-\treturn remote != remote_get(NULL);\n-}\n-\n static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch = branch_get(NULL);\n-\tint triangular = is_workflow_triangular(remote);\n+\tint triangular = remote != remote_get(NULL);\n \n \tswitch (push_default) {\n \tdefault:\n-- \n2.32.0.rc0\n\n"},{"id":"425832","messageId":"20210528201014.2175179-12-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 11/11] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:14Z","receivedAt":"2021-05-28T20:10:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now that the code has been unconvolutized and it's clear what it's\nactually doing, update the documentation to reflect that.\n\nNamely; the simple mode only barfs when working on a centralized\nworkflow, and there's no configured upstream branch with the same name.\n\nCc: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n Documentation/config/push.txt | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config/push.txt b/Documentation/config/push.txt\nindex f2667b2689..632033638c 100644\n--- a/Documentation/config/push.txt\n+++ b/Documentation/config/push.txt\n@@ -24,15 +24,14 @@ push.default::\n \n * `tracking` - This is a deprecated synonym for `upstream`.\n \n-* `simple` - in centralized workflow, work like `upstream` with an\n- added safety to refuse to push if the upstream branch's name is\n- different from the local one.\n+* `simple` - pushes the current branch with the same name on the remote.\n +\n-When pushing to a remote that is different from the remote you normally\n-pull from, work as `current`. This is the safest option and is suited\n-for beginners.\n+If you are working on a centralized workflow (pushing to the same repository you\n+pull from, which is typically `origin`), then you need to configure an upstream\n+branch with the same name.\n +\n-This mode has become the default in Git 2.0.\n+This mode is the default since Git 2.0, and is the safest option suited for\n+beginners.\n \n * `matching` - push all branches having the same name on both ends.\n This makes the repository you are pushing to remember the set of\n-- \n2.32.0.rc0\n\n"},{"id":"425833","messageId":"20210528201014.2175179-11-felipe.contreras@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"[PATCH 10/11] push: flip !triangular for centralized","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T20:10:13Z","receivedAt":"2021-05-28T20:10:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Git is a *distributed* version control system, centralized workflows are\nthe uncommon case, where we do need to do extra checks.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 8ecfbe8d63..1856f62861 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -186,7 +186,7 @@ static const char message_detached_head_die[] =\n \t \" git push %s HEAD:\\n\");\n \n static void setup_push_upstream(struct remote *remote, struct branch *branch,\n-\t\t\t\tint triangular)\n+\t\t\t\tint centralized)\n {\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n@@ -201,7 +201,7 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \tif (branch->merge_nr != 1)\n \t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n \t\t \"refusing to push.\"), branch->name);\n-\tif (triangular)\n+\tif (!centralized)\n \t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n \t\t \"your current branch '%s', without telling me what to push\\n\"\n \t\t \"to update which remote branch.\"),\n@@ -210,12 +210,12 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n }\n \n-static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n+static void setup_push_simple(struct remote *remote, struct branch *branch, int centralized)\n {\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n-\tif (!triangular) {\n+\tif (centralized) {\n \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n@@ -238,7 +238,7 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch = branch_get(NULL);\n-\tint triangular = remote != remote_get(NULL);\n+\tint centralized = remote == remote_get(NULL);\n \n \tswitch (push_default) {\n \tdefault:\n@@ -248,15 +248,15 @@ static void setup_default_push_refspecs(struct remote *remote)\n \n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tsetup_push_simple(remote, branch, triangular);\n+\t\tsetup_push_simple(remote, branch, centralized);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote, branch, triangular);\n+\t\tsetup_push_upstream(remote, branch, centralized);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tsetup_push_simple(remote, branch, 1);\n+\t\tsetup_push_simple(remote, branch, 0);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_NOTHING:\n-- \n2.32.0.rc0\n\n"},{"id":"425835","messageId":"CABPp-BFPXWy0GwZwT0weNs1DKMKFC4ds2tLJWcVQBOKxPMomFQ@mail.gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-4-felipe.contreras@gmail.com","subject":"Re: [PATCH 03/11] push: reorganize setup_push_simple()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T20:52:19Z","receivedAt":"2021-05-28T20:52:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n wrote:\n>\n> Simply move the code around.\n\nNot quite, you also deleted dead code. Made the patch a bit harder to\nread because I was trying to verify you did what the commit message\nsaid and it took me longer than it should have to realize that you\nwere also deleting dead code. Might be worth including that fact in\nthis sentence here.\n\n> No functional changes.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 29 ++++++++++++-----------------\n> 1 file changed, 12 insertions(+), 17 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index d173c39283..9c807ed707 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -225,13 +225,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n>\n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> + const char *dst;\n> +\n> + if (!branch)\n> + die(_(message_detached_head_die), remote->name);\n> +\n> if (triangular) {\n> - if (!branch)\n> - die(_(message_detached_head_die), remote->name);\n> - refspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> + dst = branch->refname;\n> } else {\n> - if (!branch)\n> - die(_(message_detached_head_die), remote->name);\n> if (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> die(_(\"The current branch %s has no upstream branch.\\n\"\n> \"To push the current branch and set the remote as upstream, use\\n\"\n> @@ -243,20 +244,14 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n> if (branch->merge_nr != 1)\n> die(_(\"The current branch %s has multiple upstream branches, \"\n> \"refusing to push.\"), branch->name);\n> - if (triangular)\n> - die(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> - \"your current branch '%s', without telling me what to push\\n\"\n> - \"to update which remote branch.\"),\n> - remote->name, branch->name);\n\nThis if-block is safe to delete because we're already in the !triangular case.\n\n\n> -\n> - if (1) {\n> - /* Additional safety */\n> - if (strcmp(branch->refname, branch->merge[0]->src))\n> - die_push_simple(branch, remote);\n> - }\n>\n> - refspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n> + /* Additional safety */\n> + if (strcmp(branch->refname, branch->merge[0]->src))\n> + die_push_simple(branch, remote);\n> +\n> + dst = branch->merge[0]->src;\n> }\n> + refspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n> }\n>\n> static int is_workflow_triangular(struct remote *remote)\n> --\n> 2.32.0.rc0\n\nEverything else is, as you say, just moving code around.\n"},{"id":"425836","messageId":"CABPp-BG=496bZ78ToKK0vwmmtWDDC4qbCZfXbL+odzecy8U7vw@mail.gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-5-felipe.contreras@gmail.com","subject":"Re: [PATCH 04/11] push: simplify setup_push_simple()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T20:57:33Z","receivedAt":"2021-05-28T20:57:50Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n wrote:\n>\n> branch->refname can never be different from branch->merge[0]->src.\n\nThis statement isn't true without additional qualifications. Perhaps\nextend your commit message with \"...since the 'Additional safety'\ncheck dies if they differ.\" or some other wording that qualifies why\nit's true at the point of the code in question.\n\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 10 ++--------\n> 1 file changed, 2 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 9c807ed707..73fe083682 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -225,14 +225,10 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n>\n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> - const char *dst;\n> -\n> if (!branch)\n> die(_(message_detached_head_die), remote->name);\n>\n> - if (triangular) {\n> - dst = branch->refname;\n> - } else {\n> + if (!triangular) {\n> if (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> die(_(\"The current branch %s has no upstream branch.\\n\"\n> \"To push the current branch and set the remote as upstream, use\\n\"\n> @@ -248,10 +244,8 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n> /* Additional safety */\n> if (strcmp(branch->refname, branch->merge[0]->src))\n> die_push_simple(branch, remote);\n> -\n> - dst = branch->merge[0]->src;\n> }\n> - refspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n> + refspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> }\n>\n> static int is_workflow_triangular(struct remote *remote)\n> --\n> 2.32.0.rc0\n\nSimple transformation allowed by the \"Additional safety\" check; makes sense.\n"},{"id":"425837","messageId":"CABPp-BHs_id3kOHgBWCK0QjviuRu7Dgbq5iB7z5=RZDOX-ontA@mail.gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-11-felipe.contreras@gmail.com","subject":"Re: [PATCH 10/11] push: flip !triangular for centralized","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T21:05:09Z","receivedAt":"2021-05-28T21:05:25Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n wrote:\n>\n> Git is a *distributed* version control system, centralized workflows are\n> the uncommon case, where we do need to do extra checks.\n\nThe commit message seemed slightly funny to me, though I'm not sure\nwhy. However...\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 16 ++++++++--------\n> 1 file changed, 8 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 8ecfbe8d63..1856f62861 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -186,7 +186,7 @@ static const char message_detached_head_die[] =\n> \" git push %s HEAD:\\n\");\n>\n> static void setup_push_upstream(struct remote *remote, struct branch *branch,\n> - int triangular)\n> + int centralized)\n> {\n> if (!branch)\n> die(_(message_detached_head_die), remote->name);\n> @@ -201,7 +201,7 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n> if (branch->merge_nr != 1)\n> die(_(\"The current branch %s has multiple upstream branches, \"\n> \"refusing to push.\"), branch->name);\n> - if (triangular)\n> + if (!centralized)\n> die(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> \"your current branch '%s', without telling me what to push\\n\"\n> \"to update which remote branch.\"),\n> @@ -210,12 +210,12 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n> refspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n> }\n>\n> -static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> +static void setup_push_simple(struct remote *remote, struct branch *branch, int centralized)\n> {\n> if (!branch)\n> die(_(message_detached_head_die), remote->name);\n>\n> - if (!triangular) {\n> + if (centralized) {\n> if (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> die(_(\"The current branch %s has no upstream branch.\\n\"\n> \"To push the current branch and set the remote as upstream, use\\n\"\n> @@ -238,7 +238,7 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n> static void setup_default_push_refspecs(struct remote *remote)\n> {\n> struct branch *branch = branch_get(NULL);\n> - int triangular = remote != remote_get(NULL);\n> + int centralized = remote == remote_get(NULL);\n>\n> switch (push_default) {\n> default:\n> @@ -248,15 +248,15 @@ static void setup_default_push_refspecs(struct remote *remote)\n>\n> case PUSH_DEFAULT_UNSPECIFIED:\n> case PUSH_DEFAULT_SIMPLE:\n> - setup_push_simple(remote, branch, triangular);\n> + setup_push_simple(remote, branch, centralized);\n> break;\n>\n> case PUSH_DEFAULT_UPSTREAM:\n> - setup_push_upstream(remote, branch, triangular);\n> + setup_push_upstream(remote, branch, centralized);\n> break;\n>\n> case PUSH_DEFAULT_CURRENT:\n> - setup_push_simple(remote, branch, 1);\n> + setup_push_simple(remote, branch, 0);\n> break;\n>\n> case PUSH_DEFAULT_NOTHING:\n> --\n> 2.32.0.rc0\n\n...I think the code is slightly easier to read and reason about since\nit removes the double negative. In particular, when someone reading\nthe code sees !triangular, and doesn't know or remember the meaning,\nthey have to translate that to !(remote != remote_get(NULL)).\ncentralized and !centralized do not have that same problem. So, I\nlike the newer version.\n"},{"id":"425838","messageId":"CABPp-BFsugBhk8-jkCpruFqn2W0aVjbyALwQgBYj56X9cf9vMw@mail.gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-12-felipe.contreras@gmail.com","subject":"Re: [PATCH 11/11] doc: push: explain default=simple correctly","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T21:07:07Z","receivedAt":"2021-05-28T21:07:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n wrote:\n>\n> Now that the code has been unconvolutized and it's clear what it's\n> actually doing, update the documentation to reflect that.\n>\n> Namely; the simple mode only barfs when working on a centralized\n> workflow, and there's no configured upstream branch with the same name.\n>\n> Cc: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/config/push.txt | 13 ++++++-------\n> 1 file changed, 6 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt\n> index f2667b2689..632033638c 100644\n> --- a/Documentation/config/push.txt\n> +++ b/Documentation/config/push.txt\n> @@ -24,15 +24,14 @@ push.default::\n>\n> * `tracking` - This is a deprecated synonym for `upstream`.\n>\n> -* `simple` - in centralized workflow, work like `upstream` with an\n> - added safety to refuse to push if the upstream branch's name is\n> - different from the local one.\n> +* `simple` - pushes the current branch with the same name on the remote.\n> +\n> -When pushing to a remote that is different from the remote you normally\n> -pull from, work as `current`. This is the safest option and is suited\n> -for beginners.\n> +If you are working on a centralized workflow (pushing to the same repository you\n> +pull from, which is typically `origin`), then you need to configure an upstream\n> +branch with the same name.\n> +\n> -This mode has become the default in Git 2.0.\n> +This mode is the default since Git 2.0, and is the safest option suited for\n> +beginners.\n>\n> * `matching` - push all branches having the same name on both ends.\n> This makes the repository you are pushing to remember the set of\n> --\n> 2.32.0.rc0\n\nMuch clearer. Well done.\n"},{"id":"425840","messageId":"CABPp-BGEtJqorUeQ4evWdmCvb=zsnkAdOn-ToG48=AYp3aUwNg@mail.gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-1-felipe.contreras@gmail.com","subject":"Re: [PATCH 00/11] Unconvolutize push.default=simple","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T21:17:54Z","receivedAt":"2021-05-28T21:18:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n wrote:\n\nIs \"Unconvolutize\" a convoluted synonym of simplify, untwist, or\nperhaps deconvolute? ;-)\n\n> Tired of jumping through hoops trying to understand what the \"simple\"\n> mode does, I decided to reorganize it up for good so it's crystal\n> clear.\n>\n> There are no functional changes.\n\nA couple tweaks to some commit messages would make it easier to verify\nthat you have introduced no functional changes (at least for reviewers\nlike me who aren't that familiar with this code area).\n\n> Basically the simple mode pushes the current branch with the same name\n> on the remote.\n>\n> Except... when there's no upstream branch configured with the same name.\n>\n> Now the code and the documentation are clear.\n\nI've read through the series and agree they do as you say. The code\nand documentation changes look good to me.\n\n> This has the additional advantage of making `current` based on `simple`,\n> rather than the other way around; `current` is basically `simple`\n> but assuming we are never in a centralized workflow.\n>\n> Felipe Contreras (11):\n> push: hedge code of default=simple\n> push: move code to setup_push_simple()\n> push: reorganize setup_push_simple()\n> push: simplify setup_push_simple()\n> push: remove unused code in setup_push_upstream()\n> push: merge current and simple\n> push: remove redundant check\n> push: fix Yoda condition\n> push: remove trivial function\n> push: flip !triangular for centralized\n> doc: push: explain default=simple correctly\n>\n> Documentation/config/push.txt | 13 +++++-----\n> builtin/push.c | 47 +++++++++++++++++++----------------\n> 2 files changed, 31 insertions(+), 29 deletions(-)\n>\n> --\n> 2.32.0.rc0\n"},{"id":"425841","messageId":"60b1604b66329_2a3717208eb@natae.notmuch","threadId":"55794","inReplyTo":"CABPp-BFPXWy0GwZwT0weNs1DKMKFC4ds2tLJWcVQBOKxPMomFQ@mail.gmail.com","subject":"Re: [PATCH 03/11] push: reorganize setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T21:27:39Z","receivedAt":"2021-05-28T21:27:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> wrote:\n> >\n> > Simply move the code around.\n> \n> Not quite, you also deleted dead code. Made the patch a bit harder to\n> read because I was trying to verify you did what the commit message\n> said and it took me longer than it should have to realize that you\n> were also deleting dead code. Might be worth including that fact in\n> this sentence here.\n\nOK. I thought that was obvious.\n\nShall I update the commit message to include that fact, or shall I add a\nseparate patch to remove the dead code?\n\nEither are fine by me.\n\n> > - if (triangular)\n> > - die(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> > - \"your current branch '%s', without telling me what to push\\n\"\n> > - \"to update which remote branch.\"),\n> > - remote->name, branch->name);\n> \n> This if-block is safe to delete because we're already in the !triangular case.\n> \n> > -\n> > - if (1) {\n\nTechically this is removing dead code too.\n\n-- \nFelipe Contreras\n"},{"id":"425842","messageId":"60b1609665443_2a371720853@natae.notmuch","threadId":"55794","inReplyTo":"CABPp-BG=496bZ78ToKK0vwmmtWDDC4qbCZfXbL+odzecy8U7vw@mail.gmail.com","subject":"Re: [PATCH 04/11] push: simplify setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T21:28:54Z","receivedAt":"2021-05-28T21:28:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> wrote:\n> >\n> > branch->refname can never be different from branch->merge[0]->src.\n> \n> This statement isn't true without additional qualifications. Perhaps\n> extend your commit message with \"...since the 'Additional safety'\n> check dies if they differ.\" or some other wording that qualifies why\n> it's true at the point of the code in question.\n\nWill do. That in fact was in the back of my mind.\n\n-- \nFelipe Contreras\n"},{"id":"425844","messageId":"CABPp-BG1FoBJ8-swiGHLJRqpE4iQiCNTmCi=8zzyms3bLJAcqQ@mail.gmail.com","threadId":"55794","inReplyTo":"60b1604b66329_2a3717208eb@natae.notmuch","subject":"Re: [PATCH 03/11] push: reorganize setup_push_simple()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T21:42:50Z","receivedAt":"2021-05-28T21:43:04Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 2:27 PM Felipe Contreras\n wrote:\n>\n> Elijah Newren wrote:\n> > On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> > wrote:\n> > >\n> > > Simply move the code around.\n> >\n> > Not quite, you also deleted dead code. Made the patch a bit harder to\n> > read because I was trying to verify you did what the commit message\n> > said and it took me longer than it should have to realize that you\n> > were also deleting dead code. Might be worth including that fact in\n> > this sentence here.\n>\n> OK. I thought that was obvious.\n\nIt is if you are familiar with the code, or if you still remember that\ncondition when you get to that point in the review, or if you happen\nto look at the right line of code while mulling it over.\nUnfortunately, I'm not familiar with this code, didn't happen to\nremember the outer if-block when I got to that point in the\ncomparison, and didn't at first look back to the relevant line to\nrealize this. So I started looking elsewhere for why removing that\ncondition didn't amount to functional changes and started trying to\nfigure out a testcase that would give different behavior. I suspect I\nwould have realized sooner if not for the claim that you \"simply moved\ncode around\", so I just flagged it. Not a big deal, just something\nthat I think could make it easier for other reviewers.\n\n> Shall I update the commit message to include that fact, or shall I add a\n> separate patch to remove the dead code?\n\nI'd just tweak the commit message to mention you are just deleting\ndead code and moving code around.\n\n> Either are fine by me.\n>\n> > > - if (triangular)\n> > > - die(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> > > - \"your current branch '%s', without telling me what to push\\n\"\n> > > - \"to update which remote branch.\"),\n> > > - remote->name, branch->name);\n> >\n> > This if-block is safe to delete because we're already in the !triangular case.\n> >\n> > > -\n> > > - if (1) {\n>\n> Techically this is removing dead code too.\n\nYep, true.\n"},{"id":"425845","messageId":"60b1674ca4f58_2a3717208ec@natae.notmuch","threadId":"55794","inReplyTo":"CABPp-BHs_id3kOHgBWCK0QjviuRu7Dgbq5iB7z5=RZDOX-ontA@mail.gmail.com","subject":"Re: [PATCH 10/11] push: flip !triangular for centralized","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T21:57:32Z","receivedAt":"2021-05-28T21:57:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> wrote:\n> >\n> > Git is a *distributed* version control system, centralized workflows are\n> > the uncommon case, where we do need to do extra checks.\n> \n> The commit message seemed slightly funny to me, though I'm not sure\n> why.\n\nI mean, in my mind I have present two facts:\n\n 1. Linus Torvalds explained that the whole point of Git was to have a\n decentralized VCS [1]. It's not just an important aspect; it's\n crucial.\n\n If we never get past [the part where I explain distribution], I'd be\n fine with that...\n\n If you are not distributed, you are not worth using. It's that\n simple.\n\n I think this is a huge issue and that *alone* should mean that every\n single open source [project] should never use anything but a\n distributed model...\n\n 2. Git doesn't actually have great support for \"triangular workflows\".\n\nSo I do find it funny that the boolean is called \"triangular\", when\nthat's supposed to be the whole point from the creator.\n\nMaybe some of that spills out into the rhetoric of the commit message.\n\n> ...I think the code is slightly easier to read and reason about since\n> it removes the double negative. In particular, when someone reading\n> the code sees !triangular, and doesn't know or remember the meaning,\n> they have to translate that to !(remote != remote_get(NULL)).\n> centralized and !centralized do not have that same problem. So, I\n> like the newer version.\n\nTo my mind it's not a slight thing at all. Every time I read if\n(triangular) I have to stop and translate... Oh, they mean using git in\nthe way it was the whole point of starting the project... So they\nactually mean not using it in that *other* way.\n\nCheers.\n\n[1] https://www.youtube.com/watch?v=4XpnKHJAok8\n\n-- \nFelipe Contreras\n"},{"id":"425846","messageId":"60b16d05d87e7_2a3a9a2088@natae.notmuch","threadId":"55794","inReplyTo":"CABPp-BGEtJqorUeQ4evWdmCvb=zsnkAdOn-ToG48=AYp3aUwNg@mail.gmail.com","subject":"Re: [PATCH 00/11] Unconvolutize push.default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-28T22:21:57Z","receivedAt":"2021-05-28T22:22:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Elijah Newren wrote:\n> On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> wrote:\n> \n> Is \"Unconvolutize\" a convoluted synonym of simplify, untwist, or\n> perhaps deconvolute? ;-)\n\nIndeed. Does it give you an urge to remedy that?\n\n> > Tired of jumping through hoops trying to understand what the \"simple\"\n> > mode does, I decided to reorganize it up for good so it's crystal\n> > clear.\n> >\n> > There are no functional changes.\n> \n> A couple tweaks to some commit messages would make it easier to verify\n> that you have introduced no functional changes (at least for reviewers\n> like me who aren't that familiar with this code area).\n\nI'll wait a bit for futher comments and resend with those changes.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"425848","messageId":"CABPp-BGSzgOwG0HjDG9WdytXT-x1-KBC0vipBgpS4a+X6aJ0Rw@mail.gmail.com","threadId":"55794","inReplyTo":"60b16d05d87e7_2a3a9a2088@natae.notmuch","subject":"Re: [PATCH 00/11] Unconvolutize push.default=simple","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-05-28T22:28:14Z","receivedAt":"2021-05-28T22:28:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 28, 2021 at 3:22 PM Felipe Contreras\n wrote:\n>\n> Elijah Newren wrote:\n> > On Fri, May 28, 2021 at 1:10 PM Felipe Contreras\n> > wrote:\n> >\n> > Is \"Unconvolutize\" a convoluted synonym of simplify, untwist, or\n> > perhaps deconvolute? ;-)\n>\n> Indeed. Does it give you an urge to remedy that?\n\nNah, it's fine.\n"},{"id":"425850","messageId":"540e249f-6bf9-0e4f-51db-20ce3ef8e5bf@gmail.com","threadId":"55794","inReplyTo":"20210528201014.2175179-12-felipe.contreras@gmail.com","subject":"Re: [PATCH 11/11] doc: push: explain default=simple correctly","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-29T05:38:41Z","receivedAt":"2021-05-29T05:38:49Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 29/05/21 03.10, Felipe Contreras wrote:\n> diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt\n> index f2667b2689..632033638c 100644\n> --- a/Documentation/config/push.txt\n> +++ b/Documentation/config/push.txt\n> @@ -24,15 +24,14 @@ push.default::\n> \n> * `tracking` - This is a deprecated synonym for `upstream`.\n> \n> -* `simple` - in centralized workflow, work like `upstream` with an\n> - added safety to refuse to push if the upstream branch's name is\n> - different from the local one.\n> +* `simple` - pushes the current branch with the same name on the remote.\n> +\n> -When pushing to a remote that is different from the remote you normally\n> -pull from, work as `current`. This is the safest option and is suited\n> -for beginners.\n> +If you are working on a centralized workflow (pushing to the same repository you\n> +pull from, which is typically `origin`), then you need to configure an upstream\n> +branch with the same name.\n> +\n> -This mode has become the default in Git 2.0.\n> +This mode is the default since Git 2.0, and is the safest option suited for\n> +beginners.\n> \n> * `matching` - push all branches having the same name on both ends.\n> This makes the repository you are pushing to remember the set of\n> \n\nGrammar looks OK.\n\nReviewed-by: Bagas Sanjaya \n\n-- \nAn old man doll... just what I always wanted! - Clara\n"}]}