Re: [PATCH 1/3] Documentation: use allowlist and denylist

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 13, 2022, 20:20 UTC
Message-ID: <xmqqk08gbw91.fsf@gitster.g>
In-Reply-To: <ec81aac05c40318755f5311a20e8f9cc55d289fc.1657718450.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

> @@ -313,7 +313,7 @@ circumstances, allowing easier restricted usage through git-shell.
>  
>  GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
>  
> -GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
> +GIT_CVSSERVER_ROOT specifies a single-directory allowlist. The

Nowhere before this documentation there is mention of white/allow. They consistently say "list of allowed directories".

It may probably make the resulting text much easier to read if we avoid the non-word "allowlist", which was recently invented only to avoid using the word "whitelist".

    GIT_CVSSERVER_ROOT specifies a single allowed directory.

IOW, the original did not even have to say "whitelist".

Show 10 quoted lines

> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index fdc28c041c7..ff74a90aead 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -33,7 +33,7 @@ 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.
> +the offers to a allowlist comprising of those.

"a -> an"; but I think the same suggestion as cvs-server equally applies here.

    -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.
    +for export this way (unless the `--export-all` parameter is specified).
    +You can furtherrestrict the offers by passing the list of allowed directories
    +as 'git daemon' arguments.

would be much easier to understand, with or without s/white/allow/.

Show 10 quoted lines

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 302607a4967..384718ee677 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -887,7 +887,7 @@ for full details.
>  	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
> +	allowlist, not a denylist). See the description of

As I always tell contributors in my review, whenever we see "In other words" and parenthesized follow-up explanation, we should take it as an admission by the author that whatever the author wrote before it is badly written and should have been expressed in a more clear way.

    In other words, only the protocols listed on this variable are
    allowed and all others are disallowed.  See the description of
    ...

without "(i.e....)" would be shorter and easier to read, I would think.