Re: [PATCH 3/7] maintenance: add --scheduled option and config

["Đoàn Trần Công Danh" <congdanhqx@gmail.com>]

On 2020-08-19 17:16:44+0000, Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> 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