ADR 9: cardano-api modules exports convention#63
Conversation
72d1316 to
fe9bba4
Compare
| Each top level export should export distinct symbols. | ||
| This may depend on the case, for example: `Cardano.Api.Byron` symbols which are still in use in current eras, can be reexported through `Cardano.Api`. | ||
|
|
||
| * `Cardano.Api` - everything domain related for the currently used and upcoming eras |
There was a problem hiding this comment.
Unfortunately we have turned Cardano.Api and Cardano.Api.Shelley into a dumping ground.
cardano-api/internal/Cardano/Api/* has a mass of modules. We should start by grouping modules under sensible directories. E.g any Serialise*.hs module can go into a Serialization folder. Eventually we can have several submodules with clear names.
e.g Cardano.Api.Serialization that re-exports all of the relevant serialization modules.
The distinction between Cardano.Api and Cardano.Api.Shelley is unclear. Because we have a split between Byron and all the other eras, I would suggest we eventually deprecate Cardano.Api.Shelley and use Cardano.Api as an "everything import" if possible. But not exporting what you mentioned below regarding utilities to avoid name clashes.
There was a problem hiding this comment.
Because we have a split between Byron and all the other eras, I would suggest we eventually deprecate Cardano.Api.Shelley and use Cardano.Api as an "everything import" if possible.
💯 👍🏻
| Those should **not** be reexported from `Cardano.Api`, because they would pollute symbol namespace, and are not required for using `cardano-api` core features but can be useful for the advanced users. | ||
| 1. Reexports from upstream libraries like consensus, network or ledger, also fall into this category e.g. `Cardano.Api.Ledger`. | ||
|
|
||
| Modules which are exposing general purpose classes and functions should go also into a separate module, and they not need to be reexported through `Cardano.Api`, for example: |
There was a problem hiding this comment.
they not need to be reexported through
Cardano.Api
We are in a way re-visiting the problematic question that got us here in the first place. "What should go in Cardano.Api`?
The answer in my opinion is everything. We should group common functionality under a "top level" module (see Cardano.Api.Serialization) and simply re-expose all of these "top-level" modules via Cardano.Api. For users who only want specific functionality they can pick and choose which modules they want instead of importing Cardano.Api. This also benefits us because we by default export everything we want exposed via Cardano.Api. I think we can avoid name clashes by exporting the utilities separately like you suggested.
77d2f6d to
e165f8c
Compare
|
|
||
| * Reexports from upstream libraries like consensus, network or ledger, also fall into this category e.g. `Cardano.Api.Ledger`. | ||
|
|
||
| * Modules which are exposing general purpose classes and functions should go also into a separate module, and they don't need to be reexported through `Cardano.Api`. |
There was a problem hiding this comment.
So this is the problem with an "everything" module like Cardano.Api, it can't be everything. I'd like to demarcate this via the module name more clearly. What about something like:
Cardano.Api.Direct.UTxO -- Direct import modules (not re-exported)
or overall something like:
Cardano.Api/
Core/ -- re-exported through Cardano.Api
Direct/ -- must be imported directly
Internal/ -- implementation details
There was a problem hiding this comment.
I think we only need in this case an Internal module. Everything what is exposed only through Cardano.Api should go to Cardano.Api.Internal. For example, if our current module Cardano.Api.Internal.Fees is meant to be only exposed through Cardano.Api, it should be where it's now.
If a module is meant to be a top level export, not necessarily exposed through Cardano.Api, it should be a top level module. If our current Cardano.Api.Internal.Fees is meant to be a top level export only, it should be split into two:
Cardano.Api.Fees- public APICardano.Api.Fees.Internal- internal functions, not exposed
Nothing prevents reexporting selected symbols from Cardano.Api.Fees through Cardano.Api if necessary.
I've described it in "Deeper level exports" section.
|
@Jimbo4350 Usually reexported symbols carry haddocks with them, so that should be fine. We can also create modules with just haddocks, reexporting what's needed for that purpose. @palas could you support us here with few examples of problematic modules / symbols? |
e165f8c to
2c0c053
Compare
2c0c053 to
06e26a6
Compare
No description provided.