Re: [PATCH 00/25] worktree lock, move, remove and unlock

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

On Thu, Apr 14, 2016 at 5:40 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, Apr 14, 2016 at 11:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> This is basically a resend from last time, which happened during rc
>>> time.
>>
>> It would have made them a much more pleasant read if you re-read
>> them during that time and added the missing "why" to many of the
>> commit log message.
>
> Hmm... I thought I didn't receive any comments last time.

I think you've been here long enough to know that absense of comments
does not mean anything more than that: lack of interest at that moment in time.

>> This looks parallel to die_errno(); isn't error_errno() a better name?
>
> To me, no. Duplicating the "err" looks weird. error_no() does not look
> good either. Though there's a couple of warning(..., strerror()),
> which could become warning_errno(). Then maybe error_errno() makes
> more sense because all three follow the same naming convention.

So in the end error_errno() would be a better name to you after all ;-)
I agree the stuttering sound coming from repeating error twice feels
somewhat odd, but warning_errno() would be so natural and obvious
future direction, so...

>>>   [03/25] copy.c: import copy_file() from busybox
>>>   [04/25] copy.c: delete unused code in copy_file()
>>>   [05/25] copy.c: convert bb_(p)error_msg to (sys_)error
>>>   [06/25] copy.c: style fix
>>>   [07/25] copy.c: convert copy_file() to copy_dir_recursively()
>>
>> Somewhere among these, there needs to be a single overview of why we
>> want "cp" implementation of busybox, e.g. what part of "cp" we want?
>> the whole thing?  or "because this is to be used from this and that
>> codepaths to make copy of these things, we only need these parts and
>> can remove other features like this and that?"
>
> We need directory move functionality.

Yeah, I know all that but you do not want to explain that to me only when
I asked in a mailing list response. You want to get into the habit of having
that in the log message to help reviewers, not just me, before they ask such
a question.

>> But such judgement is better done when we know what are the final
>> elements that are to be listed, i.e. closer to where new things are
>> introduced.  This is especially true, as the log messages of patches
>> leading to 21 are all sketchy and do not give the readers a good
>> birds-view picture.
>
> Well. I think all the commands are there now at the end of this
> series. So we have add, list, prune, move, remove, lock and unlock. I
> guess we can group list/add/move/remove together and the rest as
> support commands. I might add "git worktree migrate" for converting
> between worktree v0 and v1. But it's not clear yet.

Just so that there is no confusion, I am not opposed to reordering.
I was just saying that the decision on what the right ordering is can
be easier to make when the readers more or less know what the final
set of things in the set are, and because that becomes only clear when
they start reading 21, and because the log messages of patches
leading to 21 do not show the direction clearly enough, it is hard to
make that judgment at this point so early in the series.