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

[Ganesh Sittampalam]

Ganesh Sittampalam <ganesh at earth.li> added the comment:

On 28/08/2019 09:19, Ben Franksen wrote:
> 
> Ben Franksen <ben.franksen at online.de> added the comment:
> 
>> patch 587d15fd349e13c83f49e70e7e004f03d4218463
>> Author: Ganesh Sittampalam <ganesh at earth.li>
>> Date:   Wed Aug 28 00:01:56 BST 2019
>>   * Make D.P.V3.Core independent of NamedPrim/PrimPatchId
> 
> I am not yet convinced hiding of the Core.RepoPatchV3 constructors and
> using a pattern synonym is worth the overhead here. We just uncovered
> one example where we legitimately want to construct a Core.RepoPatchV3
> from a named prim outside of the Core module, forcing us to export an
> "unsafe" constructor function mkPrim, circumventing the abstraction
> barrier. How is this any better than exporting the Prim constructor in
> the first place?

mkPrim is better in pure abstraction terms because it doesn't allow
pattern-matching (and the PrimP synonym that does allow it is TestOnly).

> 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.

Ganesh

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