[darcs-devel] [patch1880] decouple RepoPatchV3 impl from NamedPrim

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

>>> For me, the "official" API has always been Darcs.Patch.Vx; anything
>>> under that are implementation details, not to be accessed from outside.
>>> Such a convention is IMO enough to ensure that invariants aren't
>>> violated, and this is how we have always handled things for V1 and V2.
>>> Has that ever been a problem in practice? Note that exposing Internal
>>> modules (with appropriate warnings attached) is nowadays common practice
>>> even for whole packages.
>>
>> I don't feel that strongly about keeping this abstract, but it seems
>> that we have an opportunity to do it and that hiding implementations
>> where possible means we are forced to stop and think about it if we
>> actually do want access to them at some point.
> 
> Actually an argument against trying to keep it abstract is that it
> inhibits refactoring within the Darcs.Patch.Vx namespace. For example we
> could plausibly move the graph-based conflict resolution code to a
> separate module as it's quite independent, but then we'd certainly have
> to export the constructors. So perhaps what we really need is to be
> stricter about enforcing the convention about not accessing
> Darcs.Patch.Vx.Blah from outside, preferably with something automatic.

I was just going to hit you with exactly this argument ;-))

Something automatic to enforce the convention would be nice. The
TestOnly trick doesn't work because of the implicit export of instances.
We could rename V3.Core to V3.Internal, that would at least communicate
the intent pretty clearly. (Though in this case we should really rename
D.R.InternalTypes to something different, otherwise we couldn't even
grep -r Internal without getting false positives).

Warning pragmas would help if we could disable them selectively.

Or perhaps a script that we run during build?

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1880>
__________________________________