{"thread":{"id":"43871","subject":"[PATCH] push: change submodule default to check","startedAt":"2016-08-17T20:49:03Z","lastAt":"2016-08-24T18:09:42Z","messageCount":5,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299571","messageId":"20160817204848.8983-1-sbeller@google.com","threadId":"43871","inReplyTo":null,"subject":"[PATCH] push: change submodule default to check","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-17T20:48:48Z","receivedAt":"2016-08-17T20:49:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When working with submodules, it is easy to forget to push the submodules.\nChange the default to 'check' if any existing submodule is present on at\nleast one remote of the submodule remotes.\n\nThis doesn't affect you if you do not work with submodules.\nIf working with submodules, there are more reports of missing submodules\nrather than the desire to push the superproject without the submodules\nto be pushed out. Flipping the default to check for submodules is safer\nthan the current default of ignoring submodules while pushing.\n\nSigned-off-by: Stefan Beller \n---\n \n Probably too late for the 2.10 release as I'd propose to keep it in master for\n quite a while before actually doing a release with this.\n \n Thanks,\n Stefan\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 3bb9d6b..479150a 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -22,7 +22,7 @@ static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+static int recurse_submodules = RECURSE_SUBMODULES_CHECK;\n static enum transport_family family;\n \n static struct push_cas_option cas;\n-- \n2.9.2.730.g525ad04.dirty\n\n"},{"id":"299573","messageId":"xmqqmvkb85sw.fsf@gitster.mtv.corp.google.com","threadId":"43871","inReplyTo":"20160817204848.8983-1-sbeller@google.com","subject":"Re: [PATCH] push: change submodule default to check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-17T21:05:35Z","receivedAt":"2016-08-17T21:05:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> If working with submodules, there are more reports of missing submodules\n> rather than the desire to push the superproject without the submodules\n> to be pushed out.\n\nI do not know how you are counting the \"more reports\" part of that\nassertion, but it is very likely that it is biased by the current\ndefault. If you flip the default, you would see more reports that\nsay \"I know I wasn't ready to push the submodule part out; don't bug\nme\".\n\nIOW, those who want to have something different always sound louder,\nbecause people who are satisified tend to stay silent.\n\n> Flipping the default to check for submodules is safer\n> than the current default of ignoring submodules while pushing.\n\nThat part of the assertion, on the other hand, is justifiable.\n\n> Signed-off-by: Stefan Beller \n> ---\n> \n> Probably too late for the 2.10 release as I'd propose to keep it in master for\n> quite a while before actually doing a release with this.\n\nI think you meant 'next' not 'master'. We have a few \"let's keep it\nin 'next' to see if people scream\" topics there already--the more\nthe merrier? ;-)\n"},{"id":"299574","messageId":"CAGZ79kbb=oL7b93W-x=dxVh2aBN-H+tg9y5o4RwbepYtijQTXg@mail.gmail.com","threadId":"43871","inReplyTo":"xmqqmvkb85sw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: change submodule default to check","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-17T21:14:11Z","receivedAt":"2016-08-17T21:14:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 17, 2016 at 2:05 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> If working with submodules, there are more reports of missing submodules\n>> rather than the desire to push the superproject without the submodules\n>> to be pushed out.\n>\n> I do not know how you are counting the \"more reports\" part of that\n> assertion, but it is very likely that it is biased by the current\n> default. If you flip the default, you would see more reports that\n> say \"I know I wasn't ready to push the submodule part out; don't bug\n> me\".\n>\n> IOW, those who want to have something different always sound louder,\n> because people who are satisified tend to stay silent.\n\nYeah I thought about this mistake and how to get real numbers, but\nI was just misled by some people on #git IRC waving hands. ;)\n\n>\n>> Flipping the default to check for submodules is safer\n>> than the current default of ignoring submodules while pushing.\n>\n> That part of the assertion, on the other hand, is justifiable.\n\nok.\n\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Probably too late for the 2.10 release as I'd propose to keep it in master for\n>> quite a while before actually doing a release with this.\n>\n> I think you meant 'next' not 'master'. We have a few \"let's keep it\n> in 'next' to see if people scream\" topics there already--the more\n> the merrier? ;-)\n\nWell we put it into next, but that are not as many people as those\nrunning master I would think, so I would want to maximize both times\nin next as well as in master, e.g. if you put it into next today (unreasonable,\nbut let's assume), then it could make it into master next week, and then be\nreleased as part of 2.10 IIUC the release schedule.\n\nI would say that is too fast. rather I'd see this patch transition from next to\nmaster just after a release, such that it lives in master for a full\nrelease cycle\nbefore being actually in a release.\n\nSo far for my line of thinking.\n\nThanks,\nStefan\n"},{"id":"300079","messageId":"CAGZ79kYxu_x2qA9fq4qAWt7GgCY3Nz51TMGQS7EbGnbm9c+kqQ@mail.gmail.com","threadId":"43871","inReplyTo":"20160818140922.GA5925@sandbox","subject":"Re: [PATCH] push: change submodule default to check","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-24T16:46:42Z","receivedAt":"2016-08-24T16:46:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 18, 2016 at 7:09 AM, Heiko Voigt wrote:\n> On Wed, Aug 17, 2016 at 02:14:11PM -0700, Stefan Beller wrote:\n>> On Wed, Aug 17, 2016 at 2:05 PM, Junio C Hamano wrote:\n>> > Stefan Beller writes:\n> [...]\n>> >> Flipping the default to check for submodules is safer\n>> >> than the current default of ignoring submodules while pushing.\n>> >\n>> > That part of the assertion, on the other hand, is justifiable.\n>>\n>> ok.\n>\n> I also think that this is a good reason to flip the default. IMO more\n> people will be annoyed by not being able to checkout a certain version\n> if someone forgets to push a submodule then people deliberately pushing\n> something with a submodule hash that is not on any remote.\n>\n> At the same time I am wondering whether it makes sense to keep this for\n> a bigger version change (like 3.0) or so? Since that is were people will\n> expect such changes. Not sure when 3.0 is planned though.\n>\n> Cheers Heiko\n\nI guess we can postpone it until 3.0, though I currently think it is not a big\nissue as it helps avoiding \"bugs in your workflow\".\n\nOn the other hand if you really want to push out the superproject without\nthe submodules, you need to adapt your behavior (i.e. set an option or\ngive a command line flag), and such breaking things we should delay\nuntil 3.0.\n\nI think I'll resend it with a proper commit message, such that we can just pick\nit up when 3.0 comes around.\n\nThanks,\nStefan\n"},{"id":"300095","messageId":"xmqq1t1eqbt2.fsf@gitster.mtv.corp.google.com","threadId":"43871","inReplyTo":"CAGZ79kYxu_x2qA9fq4qAWt7GgCY3Nz51TMGQS7EbGnbm9c+kqQ@mail.gmail.com","subject":"Re: [PATCH] push: change submodule default to check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-24T18:08:57Z","receivedAt":"2016-08-24T18:09:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> I guess we can postpone it until 3.0, though I currently think it is not a big\n> issue as it helps avoiding \"bugs in your workflow\".\n>\n> On the other hand if you really want to push out the superproject without\n> the submodules, you need to adapt your behavior (i.e. set an option or\n> give a command line flag), and such breaking things we should delay\n> until 3.0.\n>\n> I think I'll resend it with a proper commit message, such that we can just pick\n> it up when 3.0 comes around.\n\nA change that needs to wait until a major version bump implies that\nit is possibly compatibility breaking. So \"resend IT\", implying one\nsingle step, sounds like a bad sign that the users won't have any\ntransition period. Shouldn't we do the usual two-step deprecation\nprocess, i.e. warn when an unconfigured user pushes a superproject\nthat may be ahead of a submodule about upcoming planned default\nchange with the first patch, and then flip the default in the second\npatch while dropping the warning?\n"}]}