On 2020-08-19 17:16:44+0000, Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote:
> +static void fill_schedule_info(struct maintenance_task *task,
> + const char *config_name,
> + timestamp_t schedule_delay)
> +{
> + timestamp_t now = approxidate("now");
I see this pattern in both previous patch and this patch,
should we create a helper (if not exist) to get current timestamp
instead, parsing "now" every now and then is not a good idea, in my
very opinionated opinion.
> + char *value = NULL;
> + struct strbuf last_run = STRBUF_INIT;
> + int64_t previous_run;
> +
> + strbuf_addf(&last_run, "maintenance.%s.lastrun", task->name);
> +
> + if (git_config_get_string(last_run.buf, &value))
> + task->scheduled = 1;
> + else {
> + previous_run = git_config_int64(last_run.buf, value);
> + if (now >= previous_run + schedule_delay)
> + task->scheduled = 1;
> + }
> +
> + free(value);
> + strbuf_release(&last_run);
> +}
> +
> static void initialize_task_config(void)
> {
> int i;
> @@ -1359,6 +1387,7 @@ static void initialize_task_config(void)
>
> for (i = 0; i < TASK__COUNT; i++) {
> int config_value;
> + char *config_str;
>
> strbuf_setlen(&config_name, 0);
> strbuf_addf(&config_name, "maintenance.%s.enabled",
> @@ -1366,6 +1395,20 @@ static void initialize_task_config(void)
>
> if (!git_config_get_bool(config_name.buf, &config_value))
> tasks[i].enabled = config_value;
> +
> + strbuf_setlen(&config_name, 0);
It looks like we have a simple and better named alias for this:
strbuf_reset(&config_name)
_reset has 400+ occurences in this code base, compare to 20 of _setlen
--
Danh
