{"thread":{"id":"45015","subject":"[PATCH] doc: add note about ignoring --no-create-reflog","startedAt":"2017-02-01T22:08:04Z","lastAt":"2017-02-01T23:54:29Z","messageCount":10,"participants":["cornelius.weig@tngtech.com","Junio C Hamano","Jeff King","Cornelius Weig"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"310665","messageId":"20170201220727.18070-1-cornelius.weig@tngtech.com","threadId":"45015","inReplyTo":null,"subject":"[PATCH] doc: add note about ignoring --no-create-reflog","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-01T22:07:27Z","receivedAt":"2017-02-01T22:08:04Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig \n\nThe commands git-branch and git-tag accept a `--create-reflog` argument.\nOn the other hand, the negated form `--no-create-reflog` is accepted as\na valid option but has no effect. This silent noop may puzzle users.\n\nTo communicate that this behavior is intentional, add a short note in\nthe manuals for git-branch and git-tag.\n\nSigned-off-by: Cornelius Weig \n---\n\nNotes:\n In a previous discussion () it\n was found that git-branch and git-tag accept a \"--no-create-reflog\" argument,\n but it has no effect, does not produce a warning, and is undocumented.\n\n Documentation/git-branch.txt | 1 +\n Documentation/git-tag.txt | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 1fae4ee..fca3754 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -91,6 +91,7 @@ OPTIONS\n \tbased sha1 expressions such as \"@\\{yesterday}\".\n \tNote that in non-bare repositories, reflogs are usually\n \tenabled by default by the `core.logallrefupdates` config option.\n+\tThe negated form `--no-create-reflog` is silently ignored.\n \n -f::\n --force::\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 5b2288c..b0b933e 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -152,6 +152,7 @@ This option is only applicable when listing tags without annotation lines.\n --create-reflog::\n \tCreate a reflog for the tag. To globally enable reflogs for tags, see\n \t`core.logAllRefUpdates` in linkgit:git-config[1].\n+\tThe negated form `--no-create-reflog` is silently ignored.\n \n ::\n \tThe name of the tag to create, delete, or describe.\n-- \n2.10.2\n\n"},{"id":"310670","messageId":"xmqq4m0do86p.fsf@gitster.mtv.corp.google.com","threadId":"45015","inReplyTo":"20170201220727.18070-1-cornelius.weig@tngtech.com","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-01T22:30:38Z","receivedAt":"2017-02-01T22:30:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"cornelius.weig@tngtech.com writes:\n\n> From: Cornelius Weig \n>\n> The commands git-branch and git-tag accept a `--create-reflog` argument.\n\nFor the purpose of contrasting the above with \"--no-create-reflog\",\nI find it a bit too weak to just say \"accept\". How about\n\n The commands git-branch and git-tag accept a `--create-reflog`\n option, and creates reflog even in a repository where\n core.logallrefupdates configuration is set not to.\n\nor something? After all \"--no-create-reflog\" is accepted. It just\ndoes not override the configured (or unconfigured) default.\n\n> On the other hand, the negated form `--no-create-reflog` is accepted as\n> a valid option but has no effect. This silent noop may puzzle users.\n\nTrue, very true.\n\n> To communicate that this behavior is intentional, add a short note in\n> the manuals for git-branch and git-tag.\n\nHmph. The added \"short note\" merely states the fact; it does not\nhint that it is intentional or it explains what reasoning is behind\nthat intention.\n\n> Signed-off-by: Cornelius Weig \n> ---\n>\n> Notes:\n> In a previous discussion () it\n> was found that git-branch and git-tag accept a \"--no-create-reflog\" argument,\n> but it has no effect, does not produce a warning, and is undocumented.\n\nReading what Peff said in the thread, I do not think we actively\nwanted this behaviour; we agreed that it is merely acceptable. \n\nSo perhaps s/this behaviour is intentional/this is known/ to weaken\nthe log message? That way, when somebody else who really cares\ncomes later and finds this commit that adds explicit notes to these\nmanual pages via \"git blame\", s/he would not be dissuaded from\nmaking things better. Such an update may make it warn when\ncore.logallrefupdates is not set to false (and continue to ignore\nthe command line option), or it may make the command line option\nactually override the configured default.\n\nWith such an update to the log message, I think the patch looks\ngood.\n\nThanks.\n\n> Documentation/git-branch.txt | 1 +\n> Documentation/git-tag.txt | 1 +\n> 2 files changed, 2 insertions(+)\n>\n> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\n> index 1fae4ee..fca3754 100644\n> --- a/Documentation/git-branch.txt\n> +++ b/Documentation/git-branch.txt\n> @@ -91,6 +91,7 @@ OPTIONS\n> \tbased sha1 expressions such as \"@\\{yesterday}\".\n> \tNote that in non-bare repositories, reflogs are usually\n> \tenabled by default by the `core.logallrefupdates` config option.\n> +\tThe negated form `--no-create-reflog` is silently ignored.\n> \n> -f::\n> --force::\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index 5b2288c..b0b933e 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -152,6 +152,7 @@ This option is only applicable when listing tags without annotation lines.\n> --create-reflog::\n> \tCreate a reflog for the tag. To globally enable reflogs for tags, see\n> \t`core.logAllRefUpdates` in linkgit:git-config[1].\n> +\tThe negated form `--no-create-reflog` is silently ignored.\n> \n> ::\n> \tThe name of the tag to create, delete, or describe.\n"},{"id":"310672","messageId":"20170201223520.b4er3av67ev5m3ls@sigill.intra.peff.net","threadId":"45015","inReplyTo":"xmqq4m0do86p.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-01T22:35:21Z","receivedAt":"2017-02-01T22:35:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 01, 2017 at 02:30:38PM -0800, Junio C Hamano wrote:\n\n> > Notes:\n> > In a previous discussion () it\n> > was found that git-branch and git-tag accept a \"--no-create-reflog\" argument,\n> > but it has no effect, does not produce a warning, and is undocumented.\n> \n> Reading what Peff said in the thread, I do not think we actively\n> wanted this behaviour; we agreed that it is merely acceptable.\n> \n> So perhaps s/this behaviour is intentional/this is known/ to weaken\n> the log message? That way, when somebody else who really cares\n> comes later and finds this commit that adds explicit notes to these\n> manual pages via \"git blame\", s/he would not be dissuaded from\n> making things better. Such an update may make it warn when\n> core.logallrefupdates is not set to false (and continue to ignore\n> the command line option), or it may make the command line option\n> actually override the configured default.\n\nYeah, I'd consider it more of a \"known bug\" or \"known limitation\" than\nanything.\n\nThose can go in a separate section, but they're probably more likely to\nbe read when supplied next to the actual option.\n\n> With such an update to the log message, I think the patch looks\n> good.\n> [...]\n> > @@ -91,6 +91,7 @@ OPTIONS\n> > \tbased sha1 expressions such as \"@\\{yesterday}\".\n> > \tNote that in non-bare repositories, reflogs are usually\n> > \tenabled by default by the `core.logallrefupdates` config option.\n> > +\tThe negated form `--no-create-reflog` is silently ignored.\n\nThis might be nitpicking, but it's _not_ ignored. It still negates an\nearlier \"--create-reflog\". It is only that it does not override the\ndecision to create a reflog caused by the setting of\ncore.logallrefupdates.\n\n-Peff\n"},{"id":"310679","messageId":"xmqqmve5mrpe.fsf@gitster.mtv.corp.google.com","threadId":"45015","inReplyTo":"20170201223520.b4er3av67ev5m3ls@sigill.intra.peff.net","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-01T23:11:57Z","receivedAt":"2017-02-01T23:12:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This might be nitpicking, but it's _not_ ignored. It still negates an\n> earlier \"--create-reflog\". It is only that it does not override the\n> decision to create a reflog caused by the setting of\n> core.logallrefupdates.\n\nOK, rolling them all into one, how about this as an amend?\n\n-- >8 --\nFrom: Cornelius Weig \nDate: Wed, 1 Feb 2017 23:07:27 +0100\nSubject: [PATCH] doc: add note about ignoring '--no-create-reflog'\n\nThe commands git-branch and git-tag accept the '--create-reflog'\noption, and create reflog even when core.logallrefupdates\nconfiguration is explicitly set not to.\n\nOn the other hand, the negated form '--no-create-reflog' is accepted\nas a valid option but has no effect (other than overriding an\nearlier '--create-reflog' on the command line). This silent noop may\npuzzle users. To communicate that this is a known limitation, add a\nshort note in the manuals for git-branch and git-tag.\n\nSigned-off-by: Cornelius Weig \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-branch.txt | 3 +++\n Documentation/git-tag.txt | 3 +++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 5516a47b54..102e426fd8 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -91,6 +91,9 @@ OPTIONS\n \tbased sha1 expressions such as \"@\\{yesterday}\".\n \tNote that in non-bare repositories, reflogs are usually\n \tenabled by default by the `core.logallrefupdates` config option.\n+\tThe negated form `--no-create-reflog` does not override the\n+\tdefault, even though it overrides `--create-reflog` that appears\n+\tearlier on the command line.\n \n -f::\n --force::\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 2ac25a9bb3..fd7eeae075 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -152,6 +152,9 @@ This option is only applicable when listing tags without annotation lines.\n --create-reflog::\n \tCreate a reflog for the tag. To globally enable reflogs for tags, see\n \t`core.logAllRefUpdates` in linkgit:git-config[1].\n+\tThe negated form `--no-create-reflog` does not override the\n+\tdefault, even though it overrides `--create-reflog` that appears\n+\tearlier on the command line.\n \n ::\n \tThe name of the tag to create, delete, or describe.\n-- \n2.11.0-800-g4bf73cb6b2\n\n"},{"id":"310681","messageId":"dd0a1d56-aa39-a2a4-0ea2-cb64f79dd640@tngtech.com","threadId":"45015","inReplyTo":"xmqqmve5mrpe.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Cornelius Weig","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-01T23:19:19Z","receivedAt":"2017-02-01T23:19:28Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"\n\nOn 02/02/2017 12:11 AM, Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> This might be nitpicking, but it's _not_ ignored. It still negates an\n>> earlier \"--create-reflog\". It is only that it does not override the\n>> decision to create a reflog caused by the setting of\n>> core.logallrefupdates.\n\nThis corner case is quite important. Glad you thought about it!\n\n> -- >8 --\n> From: Cornelius Weig \n> Date: Wed, 1 Feb 2017 23:07:27 +0100\n> Subject: [PATCH] doc: add note about ignoring '--no-create-reflog'\n> \n> The commands git-branch and git-tag accept the '--create-reflog'\n> option, and create reflog even when core.logallrefupdates\n> configuration is explicitly set not to.\n> \n> On the other hand, the negated form '--no-create-reflog' is accepted\n> as a valid option but has no effect (other than overriding an\n> earlier '--create-reflog' on the command line). This silent noop may\n> puzzle users. To communicate that this is a known limitation, add a\n> short note in the manuals for git-branch and git-tag.\n> \n> Signed-off-by: Cornelius Weig \n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-branch.txt | 3 +++\n> Documentation/git-tag.txt | 3 +++\n> 2 files changed, 6 insertions(+)\n> \n> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\n> index 5516a47b54..102e426fd8 100644\n> --- a/Documentation/git-branch.txt\n> +++ b/Documentation/git-branch.txt\n> @@ -91,6 +91,9 @@ OPTIONS\n> \tbased sha1 expressions such as \"@\\{yesterday}\".\n> \tNote that in non-bare repositories, reflogs are usually\n> \tenabled by default by the `core.logallrefupdates` config option.\n> +\tThe negated form `--no-create-reflog` does not override the\n> +\tdefault, even though it overrides `--create-reflog` that appears\n> +\tearlier on the command line.\n> \n> -f::\n> --force::\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index 2ac25a9bb3..fd7eeae075 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -152,6 +152,9 @@ This option is only applicable when listing tags without annotation lines.\n> --create-reflog::\n> \tCreate a reflog for the tag. To globally enable reflogs for tags, see\n> \t`core.logAllRefUpdates` in linkgit:git-config[1].\n> +\tThe negated form `--no-create-reflog` does not override the\n> +\tdefault, even though it overrides `--create-reflog` that appears\n> +\tearlier on the command line.\n> \n> ::\n> \tThe name of the tag to create, delete, or describe.\n> \n\nYour amended version is quite concise and says everything there is to\nsay. Thanks\n"},{"id":"310682","messageId":"20170201231939.hxhhujpzyb2cqq7a@sigill.intra.peff.net","threadId":"45015","inReplyTo":"xmqqmve5mrpe.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-01T23:19:39Z","receivedAt":"2017-02-01T23:19:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 01, 2017 at 03:11:57PM -0800, Junio C Hamano wrote:\n\n> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\n> index 5516a47b54..102e426fd8 100644\n> --- a/Documentation/git-branch.txt\n> +++ b/Documentation/git-branch.txt\n> @@ -91,6 +91,9 @@ OPTIONS\n> \tbased sha1 expressions such as \"@\\{yesterday}\".\n> \tNote that in non-bare repositories, reflogs are usually\n> \tenabled by default by the `core.logallrefupdates` config option.\n> +\tThe negated form `--no-create-reflog` does not override the\n> +\tdefault, even though it overrides `--create-reflog` that appears\n> +\tearlier on the command line.\n\nShould this perhaps say \"currently\" or \"this may change in the future\",\nso that people (including those who might want to fix it later) know\nthat it's a limitation and not intentional?\n\nI'd also probably say it a little shorter, like:\n\n The negated form `--no-create-reflog` only overrides an earlier\n `--create-reflog`, but currently does not negate the setting of\n `core.logallrefupdates`.\n\nI guess that really isn't much shorter (I wondered if you could cut out\nthe \"overrides --create-reflog\" part, since that is the normal and\nexpected behavior, but I had trouble wording it to do so).\n\n-Peff\n"},{"id":"310683","messageId":"125e9d0a-7ea2-5a7e-5b5f-3bd3975b8855@tngtech.com","threadId":"45015","inReplyTo":"20170201231939.hxhhujpzyb2cqq7a@sigill.intra.peff.net","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Cornelius Weig","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-01T23:23:34Z","receivedAt":"2017-02-01T23:23:41Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"> The negated form `--no-create-reflog` only overrides an earlier\n> `--create-reflog`, but currently does not negate the setting of\n> `core.logallrefupdates`.\n\nEven better than Junio's version. I especially like that it mentions\nwhere the default setting comes from.\n"},{"id":"310685","messageId":"xmqqefzhmr02.fsf@gitster.mtv.corp.google.com","threadId":"45015","inReplyTo":"20170201231939.hxhhujpzyb2cqq7a@sigill.intra.peff.net","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-01T23:27:09Z","receivedAt":"2017-02-01T23:27:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Should this perhaps say \"currently\" or \"this may change in the future\",\n> so that people (including those who might want to fix it later) know\n> that it's a limitation and not intentional?\n\nGood point.\n\n> I'd also probably say it a little shorter, like:\n>\n> The negated form `--no-create-reflog` only overrides an earlier\n> `--create-reflog`, but currently does not negate the setting of\n> `core.logallrefupdates`.\n>\n> I guess that really isn't much shorter (I wondered if you could cut out\n> the \"overrides --create-reflog\" part, since that is the normal and\n> expected behavior, but I had trouble wording it to do so).\n\nI had the same trouble wording. Another thing I noticed was that I\ndeliberately left it vague what \"default\" this does not override,\nbecause it appears to me that those who do not set logallrefupdates\nwill get the compiled-in default and that is also not overriden.\n\nIOW, \"does not negate the setting of core.logallrefupdates\" will\nopen us to reports \"I do not have the configuration set, but I still\nget reflog even when --no-create-reflog is given\".\n\n The negated form `--no-create-reflog` currently does not negate\n the default; it overrides an earlier `--create-reflog`, though.\n\nperhaps?\n"},{"id":"310687","messageId":"20170201233202.p462dggidiiyx6s6@sigill.intra.peff.net","threadId":"45015","inReplyTo":"xmqqefzhmr02.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-01T23:32:03Z","receivedAt":"2017-02-01T23:33:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 01, 2017 at 03:27:09PM -0800, Junio C Hamano wrote:\n\n> I had the same trouble wording. Another thing I noticed was that I\n> deliberately left it vague what \"default\" this does not override,\n> because it appears to me that those who do not set logallrefupdates\n> will get the compiled-in default and that is also not overriden.\n> \n> IOW, \"does not negate the setting of core.logallrefupdates\" will\n> open us to reports \"I do not have the configuration set, but I still\n> get reflog even when --no-create-reflog is given\".\n> \n> The negated form `--no-create-reflog` currently does not negate\n> the default; it overrides an earlier `--create-reflog`, though.\n> \n> perhaps?\n\nTrue. I thought the default was \"off\", and that we merely set the config\nwhen initializing a repo. But looking again, it really is checking\nis_bare_repository() at runtime.\n\nI still think it is OK to mention, as the description of\ncore.logallrefupdates is where we document the behavior and the\ndefaults. So even with \"I do not have it set\", that is still the key to\nfind more information.\n\nI do not care that strongly either way, though. This is a minor issue,\nand I suspect just about any note would be helpful.\n\n-Peff\n"},{"id":"310689","messageId":"xmqq1svhmpqo.fsf@gitster.mtv.corp.google.com","threadId":"45015","inReplyTo":"20170201233202.p462dggidiiyx6s6@sigill.intra.peff.net","subject":"Re: [PATCH] doc: add note about ignoring --no-create-reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-01T23:54:23Z","receivedAt":"2017-02-01T23:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Feb 01, 2017 at 03:27:09PM -0800, Junio C Hamano wrote:\n>\n>> I had the same trouble wording. Another thing I noticed was that I\n>> deliberately left it vague what \"default\" this does not override,\n>> because it appears to me that those who do not set logallrefupdates\n>> will get the compiled-in default and that is also not overriden.\n>> \n>> IOW, \"does not negate the setting of core.logallrefupdates\" will\n>> open us to reports \"I do not have the configuration set, but I still\n>> get reflog even when --no-create-reflog is given\".\n>> \n>> The negated form `--no-create-reflog` currently does not negate\n>> the default; it overrides an earlier `--create-reflog`, though.\n>> \n>> perhaps?\n>\n> True. I thought the default was \"off\", and that we merely set the config\n> when initializing a repo. But looking again, it really is checking\n> is_bare_repository() at runtime.\n>\n> I still think it is OK to mention, as the description of\n> core.logallrefupdates is where we document the behavior and the\n> defaults. So even with \"I do not have it set\", that is still the key to\n> find more information.\n\nOK, let's take yours as the final and merge it down to 'next'\nsoonish.\n"}]}