brick
Make TimeZoneRegion::utc() return a TimeZoneRegion not an TimeZoneOffset
I think it's reasonable from a DX perspective to expect TimeZoneRegion::utc() to return a TimeZoneRegion instead of the offset.
@BenMorel If you could take a quick look about this one, it's pretty straight forward. It's slightly related to https://github.com/brick/date-time/issues/55
Or perhaps there shouldn't be TimeZone::utc() at all, only leaving TimeZoneOffset::utc()? That's what Java does and it makes it super clear that it's an offset-based TZ.
I appreciate the shorthand form of TimeZone::utc() and I must admit we're using it a lot in our code, so both this PR and my proposal would be a breaking change, but if it would help clear some confusion, we could live with it; it's an easy change, after all.
Good point about TimeZoneRegion::utc() returning an offset, we definitely need to fix this in one way or another.
I did like the fact than you could just get UTC from the root TimeZone class, but it definitely makes it weird when called on TimeZoneRegion.
I don't think keeping an abstract static TimeZone::utc() makes sense though, it can probably be removed (in 0.5.0).
That leaves us with two methods to represent a UTC time-zone: TimeZoneOffset::utc() and TimeZoneRegion::utc(). Isn't this confusing? What should we typically use here?
Indeed, Java has a UTC constant on ZoneOffset, but not ZoneId. Maybe doing the same would introduce less confusion? I can imagine future issues popping from users asking what's the difference between TimeZoneOffset::utc() and TimeZoneRegion::utc().
Definitely don't want to see two different utc factory. Move utc() to TimeZoneOffset looks reasonable.
Do we have any real use case for TimeZoneRegion::utc()?
@solodkiy I don't think so. You can still construct it manually with TimeZoneRegion::of('etc/UTC') anyway if required.
Moving forward, I don't think it hurts much to keep TimeZone::utc() for now, that delegates to TimeZoneOffset::utc() as it currently does.
@tigitz Can you please just add the new method in TimeZoneRegion, but not replace calls to TimeZone::utc() with TimeZoneOffset::utc()?