| Message ID | xmqqpltd4rfu.fsf@gitster.g (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | blame: do not mention obvious default configuration values | expand |
On 2024-05-22 20:44, Junio C Hamano wrote: > diff --git a/Documentation/config/blame.txt > b/Documentation/config/blame.txt > index 4d047c1790..36f965484c 100644 > --- a/Documentation/config/blame.txt > +++ b/Documentation/config/blame.txt > @@ -1,6 +1,6 @@ > blame.blankBoundary:: > Show blank commit object name for boundary commits in > - linkgit:git-blame[1]. This option defaults to false. > + linkgit:git-blame[1]. > > blame.coloring:: > This determines the coloring scheme to be applied to blame > @@ -14,11 +14,9 @@ blame.date:: > > blame.showEmail:: > Show the author email instead of author name in linkgit:git-blame[1]. > - This option defaults to false. > > blame.showRoot:: > Do not treat root commits as boundaries in linkgit:git-blame[1]. > - This option defaults to false. > > blame.ignoreRevsFile:: > Ignore revisions listed in the file, one unabbreviated object name > per Frankly, I'd much rather see the same "If true, ..." twist [1] applied here, rather than just deleting the descriptions of the defaults. To me, such an approach eliminates any doubts, while not stressing out the actual default value. [1] https://lore.kernel.org/git/54c318f42ce63f0575ea48f3dc1e8361@manjaro.org/
On Wed, May 22, 2024 at 3:13 PM Dragan Simic <dsimic@manjaro.org> wrote: > On 2024-05-22 20:44, Junio C Hamano wrote: > > diff --git a/Documentation/config/blame.txt > > b/Documentation/config/blame.txt > > @@ -1,6 +1,6 @@ > > blame.blankBoundary:: > > Show blank commit object name for boundary commits in > > - linkgit:git-blame[1]. This option defaults to false. > > + linkgit:git-blame[1]. > > @@ -14,11 +14,9 @@ blame.date:: > > Frankly, I'd much rather see the same "If true, ..." twist [1] > applied here, rather than just deleting the descriptions of the > defaults. To me, such an approach eliminates any doubts, while > not stressing out the actual default value. For what it's worth, when reviewing patches on this list, I have multiple times asked submitters to mention the default value when adding new documentation since it saves users the trouble of having to discover the default either by experimentation or by reading the source code. So, I see removal of mention of default values as a step backward.
On Wed, May 22, 2024 at 03:19:58PM -0400, Eric Sunshine wrote: > For what it's worth, when reviewing patches on this list, I have > multiple times asked submitters to mention the default value when > adding new documentation since it saves users the trouble of having to > discover the default either by experimentation or by reading the > source code. So, I see removal of mention of default values as a step > backward. FWIW, I came here to make the same comment. ;) I'd rather the documentation err on the side of being overly descriptive. -Peff
diff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt index 4d047c1790..36f965484c 100644 --- a/Documentation/config/blame.txt +++ b/Documentation/config/blame.txt @@ -1,6 +1,6 @@ blame.blankBoundary:: Show blank commit object name for boundary commits in - linkgit:git-blame[1]. This option defaults to false. + linkgit:git-blame[1]. blame.coloring:: This determines the coloring scheme to be applied to blame @@ -14,11 +14,9 @@ blame.date:: blame.showEmail:: Show the author email instead of author name in linkgit:git-blame[1]. - This option defaults to false. blame.showRoot:: Do not treat root commits as boundaries in linkgit:git-blame[1]. - This option defaults to false. blame.ignoreRevsFile:: Ignore revisions listed in the file, one unabbreviated object name per
While it helps to mention what the default behaviour is when a variable is unconfigured, when there are more than true or false choices for the variable (e.g. "defaults to never, among possible choices never/local/remote/always"), or if the default behaviour is as if the unconfigured variable were set to true the knob is purely to opt out of it ("Porcelain 'git diff' refreshes the index, but diff.autoRefreshIndex can be set to false to opt out of it"), it feels like it adds unnecesary noise to say "defaults to off" for a configuration variable that is clearly a Boolean. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * I am somewhat hesitant to use a phrase that implies that there is some "default value" for any configuration variable. Saying "The default value for the configuration variable X is Y" misleads readers to expect that "git config -l" would show X=Y when the user hasn't configured X at all. It is just the feature controlled by the configuration variable works exactly as if the configuration variable is set to that "default value", instead of left unconfigured. But that is a separate issue, and is totally outside the scope of this change. Documentation/config/blame.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)