Re: [PATCH] setup: clarify TODO comment about ignoring core.bare

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx> writes:

>  	/*
> -	 * TODO: heed core.bare from config file in templates if no
> -	 *       command-line override given
> +	 * Note: The below line simply checks the presence of worktree (the
> +	 * simplification of which is given after the line) and core.bare from
> +	 * config file is not taken into account when deciding if the worktree
> +	 * should be created or not, even if no command line override given.
> +	 * That is intentional. Therefore, if in future we want to heed
> +	 * core.bare from config file, we should do it before we create any
> +	 * subsequent directories for worktree or repo because until this point
> +	 * they should already be created.
>  	 */
>  	is_bare_repository_cfg = prev_bare_repository || !work_tree;

I do not recall the discussion; others may want to discuss if the
change above is desirable, before I come back to the topic later.

But I see this long comment totally unnecessary and distracting.

> -	/* TODO (continued):
> +	/* Note (continued):
>  	 *
> -	 * Unfortunately, the line above is equivalent to
> +	 * The line above is equivalent to
>  	 *    is_bare_repository_cfg = !work_tree;
> -	 * which ignores the config entirely even if no `--[no-]bare`
> -	 * command line option was present.
>  	 *
>  	 * To see why, note that before this function, there was this call:
>  	 *    prev_bare_repository = is_bare_repository()

If it can be proven that the assignment can be simplified to lose
the "prev_bare_repository ||" part, then the above comment can be
used as part of the proposed log message for a commit that makes
such a change.  There is no reason to leave such a long comment to
leave the more complex "A || B" expression when it can be simplified
to "B", no?

Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux