Re: [PATCH v2 2/7] maintenance: store the "last run" time in config

[Junio C Hamano <gitster@pobox.com>]

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

> I selected the timestamp before the task, as opposed to after the task,
> for a couple reasons:
>
>  1. The time the task takes to execute should not contribute to the
>     interval between running the tasks.

... as long as the run time is sufficiently shorter than the
interval, that is.  If a task takes 10-30 minutes depending on how
dirty the repository is, it does not make sense to even try to run
it every 15 minutes.

> If a daily task takes 10 minutes
>     to run, then every day the execution will drift by at least 10
>     minutes.

That is not incorrect per-se, but it does not tell us why drifting
by 10 minutes is a bad thing.

>  2. If the task fails for some unforseen reason, it would be good to
>     indicate that we _attempted_ the task at a certain timestamp. This
>     will avoid spamming a repository that is in a bad state.

Absolutely.

> +static void update_last_run(struct maintenance_task *task)
> +{
> +	timestamp_t now = approxidate("now");
> +	struct strbuf config = STRBUF_INIT;
> +	struct strbuf value = STRBUF_INIT;
> +	strbuf_addf(&config, "maintenance.%s.lastrun", task->name);
> +	strbuf_addf(&value, "%"PRItime"", now);

So is this essentially meant as a human-unreadable opaque value,
like we have in the commit object header lines?  I do not have a
strong opinion, but it would be nice to allow curious to casually
read it.  Perhaps "git config --type=timestamp maintenance.lastrun"
can be taught to pretty print its value?