Re: [PATCH 1/2] builtin/mv: remove get_pathspec()

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Aug 6, 2015 at 2:58 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Aug 6, 2015 at 11:46 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Thu, Aug 6, 2015 at 2:27 PM, Stefan Beller <sbeller@google.com> wrote:
>>> `get_pathspec` is deprecated and builtin/mv.c is its last caller, so
>>> reimplement `get_pathspec` literally in builtin/mv.c
>>
>> Curious. Since this is just moving code around, rather than doing the
>> actual work to complete the final step as stated by the NEEDSWORK
>> comment, isn't it just moving the "problem" from one location to
>> another? Is it worth the code churn?
>
> Yeah it is moving around the problem a bit. And the code churn is
> unfortunate. Though when I was reading the documentation on
> pathspecs, literally the first sentence was "Do not use get_pathspec,
> it is out dated". And that was a sad taste for reading documentation.

By loudly warning you about deprecation and, more importantly,
pointing you at the accepted alternative, this documentation saves you
from wasting time (both time spent reading and time spent going down a
dead-end path). It would be a "sad taste" if it warned you quietly
only at the end of the documentation or not at all.

> It's ok to have such warnings in the docs, but as the first sentence as if
> there was nothing more important than avoiding the out dated stuff?

>From a documentation standpoint, there is nothing more important than
warning you to avoid it since it is outdated and likely to go away in
the future.

> I mean I want to understand the actual code and how I can use it, right?

No. It's deprecated and not meant for your use.

It's a different matter if you want to understand what the function
does because you've encountered a call in existing code, but that case
is covered by the existing documentation still being intact (that is,
it wasn't removed when the deprecation notice was added).

> And there are different approaches to solving the problem.
> I could have just reworded or even just rearranged the documentation.

The documentation seems fine as-is.

> The approach I take here includes a bit of code churn, but it moves the
> problematic pieces all in one spot.

Indeed, I had the "localizing the problem to one spot" argument in
mind, and even wrote it as an answer to my own question, but deleted
it before hitting "Send". The counterargument (aside from code churn)
is, that by leaving it alone, it serves as a good reminder of the
"problem" and is more likely to get noticed and (perhaps) fixed by
someone than if it is hidden away in builtin/mv.c.