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

ZonedDateTime::{plus,minus}{Minutes,Hours,Seconds} behaves differently to Java 8 #109

Open
JanTvrdik opened this issue Jun 25, 2024 · 2 comments
Milestone

Comments

@JanTvrdik
Copy link

JanTvrdik commented Jun 25, 2024

In Java 8, for example ZonedDateTime.plusMinutes operates on the instant time-line, which means that zonedDateTime.plusMinutes(30) is the same as zonedDateTime.plus(Duration.ofMinutes(30)).

In this library, however,

$zonedDateTime->plusMinutes(30)

is different from

$zonedDateTime->plusDuration(Duration::ofMinutes(30))

If this is intentional behavior change then I think the documenation should be improved (see very good documenation of ZonedDateTime::of()).

If that is unintentional behavior change, that this may be considered a bug report.

@BenMorel
Copy link
Member

Hi @JanTvrdik,

I wasn't aware of this difference between brick/date-time and java.time, thanks for bringing it to my attention!

I think it's totally reasonable to expect this to work the same way as it does in Java, so let's consider this a bug report. I'll see if I can find some time to work on this in the coming days, unless you want to open a PR.

However, as this is a BC break, it will require a new version bump (0.8.0), as version 0.7.0 was released just 2 days ago.

@solodkiy
Copy link
Contributor

solodkiy commented Jun 26, 2024

Just to be clear:

public function plusPeriod(Period $period): ZonedDateTime
{
    return ZonedDateTime::of($this->localDateTime->plusPeriod($period), $this->timeZone);
}

public function plusDuration(Duration $duration): ZonedDateTime
{
    return ZonedDateTime::ofInstant($this->instant->plus($duration), $this->timeZone);
}

Change behavior is happening only when timezone changes between two points of time (like DST).

@BenMorel BenMorel added this to the 0.8.0 milestone Jul 6, 2024
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

3 participants