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.

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

Provide guidance on non-enumerated changes #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gbaz
Copy link
Contributor

@gbaz gbaz commented Oct 10, 2023

As inspired by #55 this PR does two things. First it changes "other changes" to "last-component changes" to disambiguate that this only covers other changes which are "extremely minor". Second, it adds a new clause for "non-enumerated changes" as an umbrella clause to handle any cases not explicitly enumerated elsewhere with the guidance that any change which would cause a package depending on the versioned package to fail to compile must be considered breaking.

This clause then can cover new features introduced by GHC as general guidance without the PVP needing to be specifically updated immediately to cover them. (Though as time goes on, we may of course add more enumerated changes from new GHC features to the explicit enumerations of breaking and non-breaking changes).

As inspired by #55 this PR does two things. First it changes "other changes" to "last-component changes" to disambiguate that this only covers other changes which are "extremely minor". Second, it adds a new clause for "non-enumerated changes" as an umbrella clause to handle any cases not explicitly enumerated elsewhere with the guidance that any change which would cause a package depending on the versioned package to fail to compile must be considered breaking.

This clause then can cover new features introduced by GHC as general guidance without the PVP needing to be specifically updated immediately to cover them. (Though as time goes on, we may of course add more enumerated changes from new GHC features to the explicit enumerations of breaking and non-breaking changes).
@hasufell
Copy link
Member

hasufell commented Oct 11, 2023

-1


  • it is not obvious to me that this is the "spirit" of the spec
  • it is a blanket statement that will regress future efforts: now we can point everything and everyone to this and stop discussing what is actually API. GHC might incorrectly stop compiling certain code changes. What causes code to stop compiling should not be mandated by current GHC implementation. And how are end users supposed to validate this? We want clear rules that can be applied without running the compiler. So either this new rule adds nothing (because it doesn't say what "would fail to compile" actually means) or it now defers PVP to a runtime property of invoking GHC

TLDR: how to figure out if you broke API: Compile all your reverse dependencies.

@gbaz
Copy link
Contributor Author

gbaz commented Oct 11, 2023

The text of the proposed change makes no mention of GHC or an implementation. The above comment does not appear to be pertinent to the text of the proposed change, but arguing against something else entirely -- what, precisely, I don't know.

@hasufell
Copy link
Member

The text of the proposed change makes no mention of GHC or an implementation.

Then it absolutely says nothing, because "would fail to compile" is entirely undefined and gives no value for the end user.

@gbaz
Copy link
Contributor Author

gbaz commented Oct 11, 2023

The intention is that the guidance is considered with regards to the specified behavior of the new features.

@@ -100,6 +100,10 @@ name is taken from another package (e.g. when `network-bytestring` was merged
into `network`) or is quite general (`Data.Set` or something similar) then the
version increase **SHOULD** be major.

1. *Non-enumerated changes*. As the Haskell language evolves, new features may be introduced (e.g. rewrite rules)
which are not explicitly covered in the above rules. In such cases, in keeping with the spirit of the PVP, any changes not
enumerated above which would cause a package depending on the versioned package to fail to compile **MUST** be considered breaking.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this a bit more, I'm not sure this is quite right. As it stands, exporting new names is a "non-breaking change", but this can cause downstream packages to not compile if they don't use explicit import lists. I think perhaps the statement we want is "unavoidably cause a downstream package to fail to compile"? I'm not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm. I think that the export of names is a sort of exception carved out on a one-off basis after a bunch of discussion (and it was controversial). This clause isn't supposed to describe anything explicitly enumerated -- its the default policy for stuff that is not otherwise covered, and there I think its better to be on the safe side, allowing for exceptions to be made only after discussion of the trade-offs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants