On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> [...]
> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index fdc28c041c7..7a0539cb411 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -32,8 +32,8 @@ that service if it is enabled.
> It verifies that the directory has the magic file "git-daemon-export-ok", and
> it will refuse to export any Git directory that hasn't explicitly been marked
> for export this way (unless the `--export-all` parameter is specified). If you
> -pass some directory paths as 'git daemon' arguments, you can further restrict
> -the offers to a whitelist comprising of those.
> +pass some directory paths as 'git daemon' arguments, the offers are limited to
> +repositories within those directories.
>
> By default, only `upload-pack` service is enabled, which serves
> 'git fetch-pack' and 'git ls-remote' clients, which are invoked
> @@ -50,7 +50,7 @@ OPTIONS
> Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
> "/foo/repo.git" or "/foo/repo/.git") and don't do user-relative paths.
> 'git daemon' will refuse to start when this option is enabled and no
> - whitelist is specified.
> + specific directories are specified.
Structurally this series should be changed so that like changes are
coupled together, this would be much easier to review with the daemon.c
changes in 3/3.
But that also shows that this change is needed, but really lacking
compared to what we could do here, which is that both the the SYNOPSIS
and the heading here should be, respectively:
[--strict-paths=<path>...]
And:
--strict-paths=<path>...:
I.e. all we're trying to get across here is "this option has a mandatory
argument", so let's just say something like that explicitly? I think in
this case we don't need the prose at all, the synopsis + heading + error
would be enough.
More generally: As I noted on v1 I think the underlying motivation for
the series is mistaken, but I'm also happy for any excuse people can
find to improve our documentation.
But as I pointed out on your similar earlier series to expunge gendered
pronouns from the docs I think best practices in our docs just happen to
align with what you want.
I.e. in that case we prefer a style that isn't introducing "actors"
anyway, so even without that motivation the prose could be improved.
And ditto here, I think the use of the term "whitelist" is fine in and
of itself, but in this case we can clearly improve the docs anyway.
I just think that the origin of the change really shows in this case, I
haven't yet reviewed the rest but suspect I'll find something
similar. I.e. there's a clear improvement to be made, but since it was
first made with s/whitelist/allowlist/g, and now an attempt to
s/whitelist//g in some form we lose sight of the larger picture.
Which in thihs case is that perhaps the sentence isn't needed at all,
and that the synopsis & title is the real thing worth fixing.
So, if what motivates doc improvements in your case is working on your
local dictionary black^Hdenylist :): great, we can probably improve our
docs in any case. But please spend a bit of time eyeballing the change
without an eye to that motivation, sometimes we can improve it much more
without much effert...
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 302607a4967..dd5061563eb 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -886,8 +886,7 @@ for full details.
> `protocol.allow` is set to `never`, and each of the listed
> protocols has `protocol.<name>.allow` set to `always`
> (overriding any existing configuration). In other words, any
> - protocol not mentioned will be disallowed (i.e., this is a
> - whitelist, not a blacklist). See the description of
> + protocol not mentioned will be disallowed. See the description of
> `protocol.allow` in linkgit:git-config[1] for more details.
>
> `GIT_PROTOCOL_FROM_USER`::
I agree with Junio's earlier feedback about "in other words" being a
telltale sign of prose that needs improving.
But the point of the previous prose (such as it was) was to elaborate on
th existing "allow" to say "oh, allow means the same as whitelist",
surely?
So I think we really could just delete this "in other words" entirely,
i.e. it's basically saying "you are allowed to eat ice cream (in other
words, you are not disallowed)", it's not adding anything anymore. The
"(...)" can just be removed.
