[MERGE] Clarify use of underscores in HACKING

[Ian Clatworthy]

John Arbash Meinel wrote:

> 3) We generally have these classes of functions:
> 
>   a) Fully public. This is intended to be a stable function that we will
> support with (at a minimum) a deprecation period.
> 
>   b) Meant to be used, but not guaranteed to be stable. _iter_changes is
> the most notable function in this category.
> 
>   c) Accessible between bzrlib modules, but not really intended to be
> used by plugins, etc.
> 
>   d) Intended to be only used within a class (not meant to be seen by
> other bzrlib classes)
> 
>   e) Not intended to be used by anyone or anything at any time, we
> didn't even realize that was there. :)
>  
> Category (b) exists because of our performance drive. We have an api
> which we *think* will fit what we want, but we aren't going to guarantee
> it, because we may need to tweak it to get it to work properly for
> performance. (If you end up needing extra information that you have to
> loop over the whole tree to get, then we need to revise the function, as
> it should be the only thing iterating the whole tree).

_iter_changes actually meets my naming expectation here. The leading _
indicates that we're aware this isn't fully cooked enough to be public
and reach class (a).

> (c) Has to do with 'friend' classes (to use the C++ term). These are
> functions that are only really meant to be used by close relatives,
> (like RevisionTree possibly accessing Repository.get_weave() so that it
> can get the text for a file). But get_weave() isn't really meant as a
> public for everyone. (Especially now that we are trying to deprecate the
> VersionedFile view of the world).

Java does this better than C++ in my opinion by supporting "package
protection" - the perfect thing we're trying to capture in (c) - public
inside bzrlib but not outside it. I still kind of feel that the right
thing in Python is to *not* have an underscore here but to either
comment it accordingly or tag it as such using some sort of decorator.

I'm not sure how flexible pydoctor and friends are in producing "just
public" API docs but I'd expect _ things to be hidden in that while I'd
expect (c) things to be exposed but marked as "for internal bzrlib use
only". Somehow.

<soapbox>
Off topic, if there's one thing that strikes me in moving from 80% Java
coding to 80% Python coding it's how infrequently different packages are
used in Python communities as a rule. bzrlib isn't small and trying to
pull apart the various layers within it was not easy when I first
started using it, my small brain and all. I don't think the "let's use
100s of packages" Java culture is quite right but the "let's put 80% of
the non-test code in 1 package" isn't either. We have *way* too many
dependencies to ever change this now but even 4-5 top level packages
reflecting best practice (DDD) architectural layering - UI, Application,
Domain, Infrastructure - helps a lot in term of comprehensibility and
dependency management IMO.
</soapbox>

> Now, because of point (1) and (2), we don't really want to use the
> '__foo' forms for functions. It also makes them very hard to whitebox
> test, because of the name mangling.

Funny how language strictness drives conventions. In Java, the way to
whitebox test is to have a parallel package tree because of precisely
this reason. Maven and other build tools expect src vs tst directories
at the top level by default.

> Which gives us a single character to indicate 4 different types of
> functions. And 2^1 < 4. We could possibly add another character into the
>  mix. But you start getting into Hungarian notation pretty quickly.

Hungarian is bad. Let's never go there.

> Otherwise, +1 on your patch. But you may want to take this discussion
> into account.

I was counting on discussion. I was arguably trolling for it by putting
the patch up. :-) Let's see if anyone else has something to say. If not,
I'll resubmit in a day or so trying to capture your 5 cases above.

Ian C.