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

condition 2 (Non-breaking changes) examples are too limited #41

Open
juhp opened this issue Apr 10, 2022 · 11 comments
Open

condition 2 (Non-breaking changes) examples are too limited #41

juhp opened this issue Apr 10, 2022 · 11 comments

Comments

@juhp
Copy link

juhp commented Apr 10, 2022

  1. Other changes. Otherwise, e.g. if change consist only of corrected documentation, non-visible change to allow different dependency range etc. A.B.C MAY remain the same (other version components may change).

Surely if I do a small tweak that improves output formatting say, that would also come under this, right?

Only mentioning documentation and dependency range changes here seems too small a class of examples to me, no?

@phadej
Copy link
Collaborator

phadej commented Apr 10, 2022

Surely if I do a small tweak that improves output formatting say, that would also come under this, right?

I'm not sure what you mean by output formatting, but I'd say no.

Only mentioning documentation and dependency range changes

Read carefully, non-visible change to allow different dependency range. That is one of a main use cases for patch versions.

The fourth digit changes shouldn't be visible outside.

That assumption is used in MIN_VERSION_pkgname macros. The downstream should not be able to distinguish between e.g. bytestring-0.10.12.0 and bytestring-0.10.12.1 (which is an example where a minor version should probably be increased to error on the safer side, as there is an addition in Data.ByteString.Internal).

@juhp
Copy link
Author

juhp commented Apr 10, 2022

I see I had formerly assumed that even a small bug would allow keeping A.B.C unchanged.

@juhp
Copy link
Author

juhp commented Sep 12, 2022

Okay but that should be made more explicit under Non-breaking change.
I really feel there is a big gap here.

Non-breaking change only talks about "new bindings, types, classes, non-orphan instances or modules (..) were added to the interface"

whereas

Other changes mentions "corrected documentation, non-visible change to allow different dependency range".

I strongly feel the scope of Non-breaking change should be expanded to mention bugfixes, output changes, etc.

Actually it would be clearer to rename Other changes to something closer to Trivial changes even I feel, but that might lead to more bike-shedding.

@juhp juhp changed the title condition 3 (Other changes) examples are too limited condition 2 (Non-breaking changes) examples are too limited Sep 12, 2022
@phadej
Copy link
Collaborator

phadej commented Sep 12, 2022

Bugfix/feature is orthogonal to breaking/non-breaking axis PVP is talking about. I argue that is an aspect where e.g. semver goes wrong. Bugfix/feature is subjective, where PVP talks about objective language-specific changes.

If bugfix require changing the context of the instance (to be more restrictive), than it's a breaking change, and no matter how you'd call it a bugfix, it still requires a major version bump.

It's also arguable whether output change is breaking change or not.. E.g. for aeson, changing the error messages did break quite a lot of tests elsewhere, so we consider changing them to be a breaking change. If PVP is going to mention behavior changes, that SHOULD mean a major bump (and people may not obey the "soft" SHOULD if there are good reasons).

@juhp
Copy link
Author

juhp commented Sep 12, 2022

I agree 1 vs 2 is more grey area, but that is not the case for 2 vs 3 though it seems.
Maybe change of behavior is better phrase than "bugfix/output" change.

@phadej
Copy link
Collaborator

phadej commented Sep 12, 2022

But I argue that change of behaviour is a breaking change.

Non-breaking changes are additions of new stuff.

@juhp
Copy link
Author

juhp commented Sep 12, 2022

So any minor bugfix release requires a major version bump?

Also I am even trying to apply pvp versioning to my non-library packages.... where I guess it makes less sense.

@phadej
Copy link
Collaborator

phadej commented Sep 12, 2022

minor bugfix release

It depends what that bugfix is. aeson moving away from HashMap can be considered a bugfix, it's barely a "feature".

@phadej
Copy link
Collaborator

phadej commented Sep 12, 2022

A recent example is mime-types-0.1.1.0. IMO, it should been 0.2 as it introduced breaking changes. Someone may try to argue that "Use 'application/xml' instead of 'text/xml'" is a bug fix, but such changes do break stuff. In this case it may very easily be observable in outer world, and break clients. I don't consider it to be safe to upgrade from 0.1.0.9 to 0.1.1.0, but PVP is written so it should be (i.e. mime-types <0.2 bound shouldn't cause me surprises, but it would!).

{- cabal:
build-depends: base, mime-types ==0.1.0.9
-}
{-# LANGUAGE OverloadedStrings #-}
module Main (main) where
import Network.Mime
main = print $ defaultMimeLookup "foo.xml"

prints "text/xml", but

{- cabal:
build-depends: base, mime-types ==0.1.1.0
-}
{-# LANGUAGE OverloadedStrings #-}
module Main (main) where
import Network.Mime
main = print $ defaultMimeLookup "foo.xml"

prints "application/xml".

@juhp
Copy link
Author

juhp commented Sep 12, 2022

Alright but "breaking" is a somewhat subjective word (I mean open to interpretation generally).
It makes me think of API changes etc, not necessarily say adding a space to some output.
So "any change of behavior" would be broader than "breaking change" it seems to me.

@phadej
Copy link
Collaborator

phadej commented Sep 12, 2022

It makes me think of API changes etc, not necessarily say adding a space to some output.

Yes, it's somewhat subjective. But there are enough real world evidence, that if something like that to be added to PVP, it SHOULD be added to Breaking changes, so people would need to think hard if their change (of behavior) can be done in a minor version, and if they are unsure they would rather make a major version (or hold on releasing the change until more changes have accumulated).

For example, a maintainer of hashable had thought hard, and explicitly said that hashWithSalt implementation may change in minor versions! However, people are still surprised by that. And some changes are akin of adding a space to some output, e.g. Hashable [a] was changed to mix in a length of a list, so ("foo", "bar") would hash to different number than ("foobar", "") - but then changes like that still broke some (arguably poorly written) tests, like comparing aeson serialized output.

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

No branches or pull requests

2 participants