haskell / pvp

Haskell Package Version Policy (PVP)
http://pvp.haskell.org/
38 stars 26 forks source link

Mention changing definitons of functions or values as Breaking change #30

Closed phadej closed 4 years ago

phadej commented 4 years ago

I propose changing

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

into (addition emphasised)

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes, classes, functions or values were changed, or orphan instances were added or any instances were removed, then ...

phadej commented 4 years ago

I think this is consistent change, but definitions of functions or values aren't mentioned anywhere. Non-breaking change talks only about addition of things (good), for example.

llelf commented 4 years ago

This doesn’t make much sense to me. Every simple refactoring (changing definitions of ⟨…⟩ functions or values), among many other things, would be a breaking change then.

phadej commented 4 years ago

I agree, it’s not a perfect wording. Yet, refactoring should not change the results functions / values produce.

Changing public x = 1 to x = 2 is breaking, but to x = y where y = succ 0 is not.

I have to study this a little myself, to see what kind of changes to functions is made on Hackage now (that’s tricky, as these changes are not visible in types!), but I hope to find a guideline.

Importantly, I think that PVP says that don’t introduce any observable, behavior change without changing any of three first version digits.

And I think that if some behaviour can be dependent upon, it will.

One example is changing the error messages. in aeson: It didn’t break anything badly, but many (more than one) test-suites broke. Similarly for trifecta we opted out to make major version bump when its error printing was changed (e.g. I do have code which tests the error messages produces by trifecta, those are visible to the user, I ”have to” be sure they are sensible).

If there were a wording about value changes, then maybe aeson would defaulted to do major bump after error refactoring.

Anyway, this is something worth clarifying. Currently PVP allows changes at any version, which is plain wrong. Breaking changes are however only SHOULD be signaled by major change, so maintainer has a freedom to not follow that (it’s not MUST) if there are good reasons

On 24 Dec 2019, at 17.37, Antonio Nikishaev notifications@github.com wrote:

This doesn’t make much sense to me. Every simple refactoring (changing definitions of ⟨…⟩ functions or values), among many other things, would be a breaking change then.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

hvr commented 4 years ago

@phadej I'd suggest to clarify the kind of changes by adding a couple more words:

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes, classes, functions or values were changed in a semantically observable way, or orphan instances were added or any instances were removed, then ...

hasufell commented 4 years ago

@hvr that introduces some ambiguity. Changing a datatype should always be a breaking change, no matter if semantics are preserved or not. I can't figure out if the current wording means that or not, so I propose:

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes, classes were changed, or orphan instances were added or any instances were removed, or the definitions of functions or values were changed in a semantically observable way, then ...

frasertweedale commented 4 years ago

So, what about I fix a little bug in the function. The type does not change, but the behaviour does change in a semantically observable way. As a library maintainer, requiring a major version bump for this does not feel right. OTOH, a major change to semantics or intended behaviour does warrant a major version bump (though perhaps a better way would be a new symbol, and deprecation of the old, otherwise users will get caught by surprise).

I support an explicit mention of this scenario in the PVP, because it is a common and important scenario that maintainers deal with. But the current proposal is too strong. I would ignore it and rely on my own judgement about whether a behavioural change requires a major version bump or not. It is better that the PVP would set a minor version bump as the minimum requirement, with an accompanying admonition to consider whether the change warrants a major version bump, and perhaps some examples.

phadej commented 4 years ago

I don't remember which particular issue triggered opening this. I don't find changing of error formatting convincing enough anymore.