Renaming identifiers and deprecating old names (withMaxSuccess/withNumTests)

[erikd]

In version 2.18 of QuickCheck the function withMaxSuccess was renamed to withNumTests with the former being immediately deprecated.

This creates a headache for large code bases that use this functionality and compies with -Werror.

It would be MUCH easier for end users if the old name was not deprecated until say 3 major releases after the rename.

[MaximilianAlgehed]

I don't quite understand this issue. The other option is (as I see it, correct me if I'm wrong) is:

• introducing the renaming in a minor version bump
• not deprecating
• most users not noticing
• deprecating later
• most users who run -Werror noticing when we deprecate
• ... those users running into the same headache?

I guess the benefit would be that the few(?) users who read the changelogs would see the new function and switch on their own volition? That would have indeed been slightly kinder on those who read the changelogs - which if that's you then kudos! We can be nicer next time :)

[erikd]

This rename was done on a major version number bump: good, thanks.

However, the issue was renaming and immediately deprecating the old name. My request is that when something is renamed, the old name is only deprecated a number of major releases later.

I support a HUGE Haskell code base, across about 8 Git repos that uses QuickCheck-2.15.* and later. These git repos, basically form a stack of dependencies, so that if I update the QC dependency to add a lower bound of >= 2.18 at the lowest level of the stack, all the layers above need this same QC dependency.

If the deprecation of withMaxSuccess had been delayed a number of major releases, the QC dependencies across the stack could have been updated in a more leisurely fashion.

Does that make sense?

[MaximilianAlgehed]

I see, the use-case you have in mind is migrating to 2.18 bottom-up and introducing the renaming one repo at a time - so uses of 2.18 downstream that haven't been migrated yet don't get the deprecation warning. That use-case was indeed not supported here.

What's causing something like git grep -lI 'withMaxSuccess' | xargs sed -i'' -e 's/withMaxSuccess/withNumTests/g' not being an option? Are you trying to support several quickcheck versions?

[erikd]

Yes, we support several QuickCheck versions because its too troublesome and not worth the effort to update them all at once.

Most of these individual git repo produce 5 or more Haskell packages which get uploaded to our own public Hackage style server. Updating all these packages all at once and uploading them to our Hackage, in sequence is probably a full week of a single engineer's time.

[lehins]

However, the issue was renaming and immediately deprecating the old name. My request is that when something is renamed, the old name is only deprecated a number of major releases later.

@erikd that is not how it is done. DEPRECATED pragma is specifically designed to do just that: deprecate a definition. There is no other way to "deprecate" a Haskell definition. Adding a comment in the haddock is not sufficient, because it is not visible enough.

For the record, marking function as DEPRECATED is no longer even considered a breaking change according to pvp-1.1: haskell/pvp#12 Unfortunately, it hasn't been released yet 😢

[Rewbert]

Is this not how DEPRECATED is supposed to work, though? Your code still works, but you're given reminders that if you want to use the new version, you should alter your code.

[sol]

@Rewbert @MaximilianAlgehed it is generally not very graceful to introduce a new name and in the same release deprecate the old name.

1. Warnings that don't fail CI are useless because people will ignore them. This is why you want/need -Werror on CI.
2. If you want to support multiple versions of QuickCheck, you now don't have any good options. You can do one of two things:
   • Ignore all deprecation warnings, which defeats the point of deprecation warnings.
   • Use CPP

For somebody who maintains a lot of Haskell code, a deprecation causes a similar level of friction as a rename without deprecation (the only difference being, that with a deprecation you will likely get a hint in the deprecation message on how to fix CI; but the code you will write to fix it will be the same in both cases; lower bound or CPP).

Also, as a user of a software I generally don't appreciate breaking changes because a breaking change does not exactly provide much user value.

[MaximilianAlgehed]

@sol one issue I see (correct me if I'm wrong) with the position you lay out here is that it's a bit self-contradictory, no? The issue is that if we introduce the new alias without a deprecation nobody will notice (a very slightly modified version of (1) in your argument), so the day we introduce the deprecation warning things will still break for people who haven't yet migrated. Furthermore, when is the right time to introduce the deprecation? By the time we could have introduced the deprecation warning we might as well remove the old change? Also, to be maximally supportive to your situation (which is also our situation with base so I really am sympathetic!) we'd basically have to postpone the deprecation (or removal) until a natural "break" happens when there is nobody left who wants to support the pre-renaming QuickCheck version (which is when, exactly?). By this argument deprecations don't serve any point at all because you'll always have this issue of supporting version with and without the deprecation, or am I missing something?

For somebody who maintains a lot of Haskell code, a deprecation causes a similar level of friction as a rename without deprecation (the only difference being, that with a deprecation you will likely get a hint in the deprecation message on how to fix CI; but the code you will write to fix it will be the same in both cases; lower bound or CPP).

Or a new module which encapsulates the deprecated function and turns off deprecation warnings locally. Which is what we did with this whole OneTuple nonsense.

Also, as a user of a software I generally don't appreciate breaking changes because a breaking change does not exactly provide much user value.

But that means not fixing previous wrongs either.

[sol]

Not going to discuss this exhaustively, but:

• Yes, I think if you introduce a breaking change you have to make a really good case for it. Something that justifies the friction it causes for downstream consumers. Personally, if I write some code then ideally, I want it to work unmodified today, tomorrow, in five years, in 10 years. Now if I depend on some early stage software, then maybe that's an unrealistic expectation. But for QuickCheck specifically, much of this code has been around for over 15 years. This means a lot of people depend on it, and people may also have taken it as just "a stable dependency, that does not change much". I think @nick8325 has done an incredible job over the years, accommodating user needs while at the same time keeping things stable.
• Looking at this particular case, by my book, it's very easy: Introduce a new name if you think it provides clarity to the user or if you "need it" for some other reason, but keep the existing name as an alias basically indefinitely (put your deprecation massage in the Haddock; move it to the end of the export list under a heading "deprecated functions").
• Another option that you always have: Use a new package name (e.g. QuickCheck3), not saying that you should, or that this is necessarily the best option, but this approach gives you ultimate freedom.

[sol]

Let me still address some of your comments:

the day we introduce the deprecation warning things will still break for people who haven't yet migrated.

The difference is, that if you do it after X amount of time, people may be able to switch over to the new name without the need for CPP and still be compatible with several older versions of QuickCheck. I think that's the point @erikd was trying to make.

@MaximilianAlgehed does that make sense? Or should I illustrator this with a concrete example or something?

Furthermore, when is the right time to introduce the deprecation?

From a users perspective, probably never. From a maintainers perspective, I think it depends on how much overhead it is to keep the deprecated alias.

Now we are all developers, and I think it's a natural urge that we want to keep things neat and tidy. I certainly know this from myself, and I have seen it from others. But this is often times what it boils down to, "an undue sense of tidiness".

[lehins]

@sol I wholeheartedly disagree with you. There is nothing wrong with a deprecation warning, which is why many people do not consider that a breaking change. If you do not use -Werror, which you should never do for a package that is released to Hackage, then your project will continue to build, albeit with a warning. Having -Werror in CI is a great idea, IMHO, which is what will force the user of a library to fix such warning, if a completely warning free project is desired. But, that should not be a requirement, if someone chooses to use -Werror they are intentionally being pedantic, and that is user's choice.

The difference is, that if you do it after X amount of time, people may be able to switch over to the new name without the need for CPP and still be compatible with several older versions of QuickCheck. I think that's the point @erikd was trying to make.

I do not believe there is ever enough X amount of time for a note in haddock of a function, which withMaxSuccess serves as a great example. Most of people that used QuickCheck for at least a few years, have become familiar enough with what this function does, so much so they will never read its haddock again, hence will never notice that there is a plan to deprecate it. That is precisely what DEPRECATED pragma is designed to solve.

To give you a concrete proof that this view is indeed shared by many Haskellers please take a look at the discussion in this PR: haskell/pvp#12

Many people wanted this in PVP for a long time, I just really hope PVP-1.1 will get released at some point:

Deprecation. Deprecated entities (via a DEPRECATED pragma) SHOULD be counted as a non-breaking change. In other words, the new A.B MAY remain the same but the new C SHOULD be greater than the old C.

[sol]

The difference is, that if you do it after X amount of time, people may be able to switch over to the new name without the need for CPP and still be compatible with several older versions of QuickCheck. I think that's the point @erikd was trying to make.

I do not believe there is ever enough X amount of time for a note in haddock of a function, which withMaxSuccess serves as a great example. Most of people that used QuickCheck for at least a few years, have become familiar enough with what this function does, so much so they will never read its haddock again, hence will never notice that there is a plan to deprecate it.

Lately, I have repeatedly seen people misinterpreting things and bending facts so that it fits their narrative.

@lehins I'm not at all implying that this is what's happening here. I probably just did a bad job in explaining something that I assumed is obvious.

• A user does not have to read the Haddocks for this to be a more graceful migration path.

If this is not obvious, and if you are genuinely interested, then I am more than happy to illustrated it with an example.

BUT: If this were one of those situations where it's more about "who is wrong" then about understanding a genuine user perspective, then I'm not interested in having this discussion.

[lehins]

@sol No, I totally understand what you mean by this example:

Looking at this particular case, by my book, it's very easy: Introduce a new name if you think it provides clarity to the user or if you "need it" for some other reason, but keep the existing name as an alias basically indefinitely (put your deprecation massage in the Haddock; move it to the end of the export list under a heading "deprecated functions").

or this:

Personally, if I write some code then ideally, I want it to work unmodified today, tomorrow, in five years, in 10 years.

And I do agree with that being a good software development practice. I just don't necessarily agree with this:

it is generally not very graceful to introduce a new name and in the same release deprecate the old name.

Sometimes when the naming is so bad (like naming of functions in QuickCheck) or functionality is so bad (like head in base), in my opinion it is OK to produce a warning.

In concrete terms, I am not in favor of removing withMaxSuccess function, ever, I am arguing that marking it as DEPRECATED, does not or should not be considered as a breaking change in the codebase and should be considered as a totally graceful migration path.

If this is not obvious, and if you are genuinely interested, then I am more than happy to illustrated it with an example.

That being said, if you believe that I misunderstood what are trying to say, then I'll be happy to see the example. FTR. I am not trying to argue about who is wrong or who is right. This whole issue is very much opinion based and I don't believe there is one right answer that would make everyone happy.

[Bodigrim]

I don't really see the issue with deprecation, that's exactly what it is for. A haddock comment won't be ever read by anyone, so the drama will repeat itself once an entity finally receives {-# DEPRECATED #-}, making it no better than slapping {-# DEPRECATED #-} from the get go. If it bothers you more than normal users because you enabled -Werror, you can set -Wwarn=deprecations or even -Wno-deprecations in an affected module.

Normally I'd consider it an overkill, but one thing which can help users who don't want to disable all deprecations even locally is to do {-# DEPRECATED in "x-quickcheck" withMaxSuccess "This is deprecated" #-}, so that they can disable only x-quickcheck deprecations with -Wno-x-quickcheck.