Re: [PATCH v2 2/2] wt-status: tolerate dangling marks

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

 



Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:

>> For example, what is the reasoning behind making dwim_ref() unable
>> to ask the "do so gently" variant, while allowing repo_dwim_ref()
>> to?
>> 
>> I am NOT necessarily saying these two functions MUST be able to
>> access the same set of features and the only difference between them
>> MUST be kept to the current "repo_* variant can work on an arbitrary
>> repository, while the variant without repo_* would work on the
>> primary repository only".  As long as there is a good reason to make
>> their power diverge, it is OK---I just do not see why and I'd like
>> to know.
>
> Maybe add the following to the end of the last paragraph of the commit
> message:
>
>   (dwim_ref() is unchanged because I expect more and more code to use
>   repo_dwim_ref(), and to reduce the size of the diff.)

If that is the reasoning, I totally disagree.  We may be staring
problems in submodules too long to think everybody should use the
variant with repo_ prefix, but a single repository operations will
continue to exist, and when you know you only need to access the
current repository, not having to use the function with longer name
without having to pass an extra parameter is a plus.

I even wonder why dwim_ref() is not defined like so in a header:

	#define dwim_ref(s, l, o, r) \
		repo_dwim_ref(the_repository, (s), (l), (o), (r))

Which would force a change like the one we are discussing to keep
them in parallel and keep the promise that only difference between
the two is that dwim_ref() works with the_repository and the other
can take an arbitrary repository.  Perhaps that can be a preliminary
clean-up patch before these two patches ;-)

Doing so would mean that coders have to remember one fewer thing
than "dwim_ref(), not only cannot take a repository pointer, cannot
be told to report error gently".  The worst part is that we know
that the difference will GROW, because the purpose of adding the
extra option argument to the API is exactly to make it easy to do
so.

Reducing the size of the diff is a good justification only when the
end result is the same.  If it burdens future developers more, that
is "I write less at the expense of forcing others to write more",
which is not quite the same thing.

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