Re: [PATCH v2 2/4] maintenance: include 'cron' details in docs

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 11, 2020, 07:10 UTC
Message-ID: <CAPig+cReatZ-iSLWreQqYLM7qONWvBLbDUNVH1dJV37VnmPBgw@mail.gmail.com>
In-Reply-To: <709a17372099351af3e3b9f2dddcd9ce220a9fcb.1604520368.git.gitgitgadget@gmail.com>

On Wed, Nov 4, 2020 at 3:06 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 10 quoted lines

> Advanced and expert users may want to know how 'git maintenance start'
> schedules background maintenance in order to customize their own
> schedules beyond what the maintenance.* config values allow. Start a new
> set of sections in git-maintenance.txt that describe how 'cron' is used
> to run these tasks.
>
> This is particularly valuable for users who want to inspect what Git is
> doing or for users who want to customize the schedule further. Having a
> baseline can provide a way forward for users who have never worked with
> cron schedules.

A few comments below, not necessarily worth a re-roll...

Show 6 quoted lines

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
> @@ -218,6 +218,60 @@ Further, the `git gc` command should not be combined with
> +The standard mechanism for scheduling background tasks on POSIX systems
> +is `cron`. This tool executes commands based on a given schedule. The

It is common in Git (and other Unix) documentation to refer to a Unix tool by its man page reference. So, for instance, instead of `cron`, we would say cron(8) since the `cron` man page is in section 8 of the Unix manual.

> +The `<path>` string is loaded to specifically use the location for the

The word "loaded" sounds odd in this context.

> +`git` executable used in the `git maintenance start` command. This allows
> +for multiple versions to be compatible. However, if the same user runs
> +`git maintenance start` with multiple Git executables, then only the
> +latest executable will be used.

I had to read this paragraph four or five times to understand what it is trying to say (assuming I do understand it). Perhaps it can be rewritten more succinctly something like this:

    The `crontab` entry specifies the full path of the `git`
    executable to ensure that the `git` command run is the same one
    with which `git maintenance start` was issued independent of
    `PATH`.

> +These commands use `git for-each-repo --config=maintenance.repo` to run
> +`git maintenance run --schedule=<frequency>` on each repository listed in
> +the multi-valued `maintenance.repo` config option. These are typically
> +loaded from the user-specific global config located at `~/.gitconfig`.

I wonder if mentioning `~/.gitconfig` explicitly is wise since it might also be at $XDG_CONFIG_HOME/git/config or some other location on Windows. Perhaps it would be sufficient to mention only "global Git configuration" or something.

Show 6 quoted lines

> +If the config values are insufficient to achieve your desired background
> +maintenance schedule, then you can create your own schedule. If you run
> +`crontab -e`, then an editor will load with your user-specific `cron`
> +schedule. In that editor, you can add your own schedule lines. You could
> +start by adapting the default schedule listed earlier, or you could read
> +https://man7.org/linux/man-pages/man5/crontab.5.html[the `crontab` documentation]

For consistency with other parts of Git documentation, it might be better to reference the `crontab` man page simply as crontab(5) rather than providing an explicit URL.

> +for advanced scheduling techniques. Please do use the full path and
> +`--exec-path` techniques from the default schedule to ensure you are
> +executing the correct binaries in your schedule.