search

brick / date-time

Date and time library for PHP
MIT License
321 stars 29 forks source link

YearMonth: deprecate int argument for month, add support for Month enum. #96

Closed gnutix closed 3 months ago

BenMorel commented 3 months ago

Side note: I was thinking of removing getMonth() in the next release (0.7), to re-introduce it later in 0.8 as returning a Month enum.

The other option would be to change the signature right away in 0.7, but I feel like skipping a version makes it more likely for people upgrading one version at a time to notice the change. Do you have an opinion on this?

gnutix commented 3 months ago

Side note: I was thinking of removing getMonth() in the next release (0.7), to re-introduce it later in 0.8 as returning a Month enum.

The other option would be to change the signature right away in 0.7, but I feel like skipping a version makes it more likely for people upgrading one version at a time to notice the change. Do you have an opinion on this?

I have no experience releasing open source libraries. But if it's a BC Break, shouldn't it be done in a new major version, like 1.0 ? I was wondering why you don't have such a version yet, don't you think this library is stable / production-ready ? (I do)

Anyway, if you still want to release 0.x version and 0.6 had a lot of BC Breaks, I (as a user) wouldn't be surprised that 0.7 has some as well. So IMHO, no need to do it in two versions, it would just be annoying having to change our code multiple times.

BenMorel commented 3 months ago

But if it's a BC Break, shouldn't it be done in a new major version, like 1.0 ? I was wondering why you don't have such a version yet, don't you think this library is stable / production-ready ? (I do)

semver allows breaking changes in version 0.x. I keep non-breaking changes in patch releases, and breaking changes in minor releases.

The library is definitely production quality, but I feel like the API is not stable yet. Once we reach the 1.0.0 release, we will have to release a major version for even the smallest of breaking changes. If I had released 1.0.0 from the beginning, we would be at version 6.0.0 already, with sometimes very few minor/patch releases in between major versions.

Tagging 1.0.0 one day, will mean that I consider the library mostly finished and the API stable.

Sure, didn't thought of the == here. There should be a unit test covering that!

Adding a unit test would mean that we support this use case, and I'm personally against using objects this way. Loose comparison is an unfortunate "feature" of PHP, which exposes object internals, and supporting it prevents using features like internal cache / lazy loading. We're a bit in a grey zone here, where I want to avoid the BC break if possible, but still not officially support this use case.

BenMorel commented 3 months ago

Thank you, @gnutix!