Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Formalize ".Internal" module convention #46

Closed
wants to merge 1 commit into from
Closed

Formalize ".Internal" module convention #46

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion v1.0/pvp-specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ ordering of the components. For example, 2.0.1 \> 1.3.2, and 2.0.1.0 \> 2.0.1.
version number. When a package is updated, the following rules govern
how the version number must change relative to the previous version:

1. *Breaking change*. If any entity was removed, or the types of any entities
1. *Internal modules*. If a change only touches entities defined in modules
whose names include the string `.Internal` then *A.B* **MAY** remain unchanged.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this is somewhat ambiguous.

  1. what about Foo.Internal.Bar?
  2. what about Foo.InternalBar?

Both include the string .Internal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, perhaps appealing to the module hierarchy instead of modules names as strings would be more appropriate here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a package is called Internal? Do we just say "don't do that then", or should it be a non-initial component?

The way I see it, we would want both AwesomeLibrary.Datatypes.PriorityQueue.Internal and GHC.Internal.ByteMunging to be considered internals, but Internal.Datatypes.List to not be.

Perhaps it's OK to just lose that namespace, though - it would be pretty confusing.

Copy link
Member

@hasufell hasufell Nov 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's exactly what's problematic with magic names.

PVP already implicitly makes a connection to cabal file syntax. So what if we we add support for "internal" modules to cabal first and then adjust PVP to reference it.

E.g.

  • exposed-modules (exposed and PVP compliant)
  • other-modules (not exposed)
  • internal-modules (exposed, but exempt from PVP)

This also allows easy static analysis and doesn't depend on module names.

It could be argued PVP is more general than haskell, but reading the spec carefully, that doesn't seem to be the case anyway.

The spec could also leave this open and intentionally vague and just point out that breaking changes can be exempted by a language's metadata format, specifying those modules in an appropriate format.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what if we we add support for "internal" modules to cabal first and then adjust PVP to reference it.

This seems to me to be a great thing to have done years ago. But today, we happen to have a spec that says to do one thing, and a lot of people who actually do something not quite what the spec says. Propagating a new file format to do this means convincing everyone to abandon their current practices, and move to something else, as well as waiting for the new versions of the various tools to propagate and spread.

Why not formalize what people actually do, which seems to work reasonably well in practice today?

Copy link
Collaborator

@phadej phadej Nov 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not formalize what people actually do, which seems to work reasonably well in practice today?

Because they don't. People treat .Internal modules of bytestring or text as stable and under PVP. Please address that specifically. (And servant fwiw).

EDIT: In particular I have never seen a client using Internal modules specifying minor version bounds. They all use wide major bounds e.g. bytestring >=0.10.3.0 && <0.12.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not formalize what people actually do, which seems to work reasonably well in practice today?

Because that's how specs generally deteriorate and simply become documentation of current (good and bad) practices.

I'd rather work towards making the spec more general and leave space for languages and tooling to decide on non-critical things.

I also don't see any particular time pressure here. Are we in a rush?


3. *Breaking change*. If any entity was removed, or the types of any entities
or the definitions of datatypes or classes were changed, or orphan instances
were added or any instances were removed, then the new *A.B* **MUST** be
greater than the previous *A.B*. Note that modifying imports or depending on a
Expand Down