deprecate the date module

[Tieske]

Considering the problems with the date module, and other modules out there that do a better job at it, I think we should be deprecating the module.

This issue is created to replace the following issues: #89, #93, #181, #180

See https://luarocks.org/search?q=date for alternatives.

EDIT: added link for alternative

[alerque]

So what's the plan?

My idea would be to...

1. Throw out our tests for it.
2. Add a big deprecation warning to STDOUT when the module is loaded with a deprecation notice and some pointers towards other modules that could be used.
3. Announce the deprecation in the next release notes with some timeline for removing it.
4. After whatever release number or timeline that was announced for removal, remove the substance of the module and upgrade the warning to an error while still pointing to alternatives in the error message.
5. After the dust settles from that, drop the module completely.

[Tieske]

I'd like to stick to SemVer, so version 2.0 would be the one to drop support for this module. In the next minor release we can add the deprecation warning and mark it as deprecated.

Alternatively start printing the warning in 2.0, and drop it from 3.0. This is probably the best fit from user expectations

[alerque]

I'm with you on SemVer.

Unfortunately SemVer doesn't have much to say on how to handle deprecation announcements. It's pretty clear how to version known-breaking changes vs new features and refactoring vs bug fixes. It's not so clear on timelines for when to to announce and warn.

For example I don't think there would be any SemVer justification to do a major release to announce a future deprecation and start warning. That in itself isn't a breaking change. Yes I agree that might be a closer fit to user expectations, but that's just because peoples vague impressions of numbers are weird — it's not part of SemVer.

My suggestion is to add the deprecation warning messages to master ASAP so that by the next minor release as many people as possible are being warned. At the very least in 1.10.0 loading the date module (and probably the XML module, see #311) should be throwing deprecation warnings.

After that we're free to wait as long as we want before actually removing them. We can have as many 1.y.z releases as we feel like (as long as we're not breaking other API expectations) before cutting a 2.0.0 that finally removes the code entirely.

[Tieske]

the problem I see with this is that a patch release suddenly printing warnings might also be considered a breaking change. It could mangle output that is relied upon.

Hence my proposal above to be more cautious, add the warning to a next major. Despite that meaning having to drag along old code until yet another major. That said, I am generally not very hesitant in breaking code, and jump to a next major, but in this case, Penlight being a utility library, it quickly would lead to a single app depending on multiple Penlight versions (eg. app depends on 2 libs, each of these libs depends on a different Penlight-major).

All thing considered, I think we should add the warning in next major, and remove it a major after that.

[alerque]

Okay. I tend to think that if people's use of Penlight is going to break if something changes on STDOUT then the chances are they have done something wrong. But I'm okay with this plan as long as we aren't shy about bumping the release counter fairly aggressively.

I'll work on some warnings, and lets think about what else should land before 2.0.

[Tieske]

outputing warnings is one thing, having a setting to suppress those is what I'm wondering how to implement?

[alerque]

Interesting idea. We'd have to think carefully about what the setting was (I hate global variables, but this might be a use-case for them) but perhaps we could implement this deprecation warning now behind a setting that defaulted to "off". That would allow us to announce it in the next release even if that is a point-release and test it ourselves without using a branch. Then whenever the next major release is we can default deprecation warnings to "on", revealing the warning that is already in place. Perhaps we can even announce that future deprecation notices sent to STDERR will not be considered breaking changes so that we don't need 2 major release cycles for future deprecation(s), only one.

[Tieske]

iirc most modules rely on pl.utils which relies on pl.compat so that would be the most obvious place imo.

As for a whishlist 😄 ;

• add a function pl.compat.deprecation_warning(msg, trace) that defaults to printing to STDERR
• that function can be overridden by applications (eg. log it in nginx log files for an OpenResty app)
• internal function pl.compat.raise_deprecation(msg) that will;
  • prefix msg with "[Penlight warning]" (probably add the Penlight version in there?)
  • add a stacktrace to the msg and then call pl.compat.deprecation_warning("[Penlight warning] " .. msg, trace)
  • do nothing if pl.compat.deprecation_warning == nil
• every deprecated element (function or module) will call the internal function

for example

function stringx.islower(str)
  deprecation_warning("'stringx.islower' has been renamed to 'stringx.is_lower'")
  return stringx.is_lower(str)
end

wdyt @alerque ?

[alerque]

Sounds good to me. The only thing I might add is arguments to the warning function for what version added the warning (for "was deprecated in X" output) and for the expected removal version ("and will be completely removed in Y").

[Tieske]

I took a stab at this: #361