search

brick / date-time

Date and time library for PHP
MIT License
334 stars 30 forks source link

Add toISOString() methods #87

Closed gnutix closed 1 year ago

gnutix commented 1 year ago

I removed Month::getName() and DayOfWeek::toISOString(). PR is ready for merge IMHO.

gnutix commented 1 year ago

@BenMorel Could you merge #85 first, and then I'll rebase this PR on master and do the changes you requested ? (otherwise if I move __toString()'s implementation in toString() first and then rebase, I'll get tons of conflicts)

BenMorel commented 1 year ago

@gnutix I was actually planning to do the opposite, as I haven't reviewed #85 yet!

Plus, you said in #85 that you needed a bug fix from here?

gnutix commented 1 year ago

All the review comments have been fixed. I think I found a way to do the changes without having to worry about conflicts... we'll see.

Plus, you said in https://github.com/brick/date-time/pull/85 that you needed a bug fix from here?

I cherry-picked the same bugfix commit in both branches. Whichever gets merged first puts the commit into master, and the other one will drop it upon rebase.

BenMorel commented 1 year ago

Thank you, @gnutix! I'm slightly concerned about YearMonthRange::toISOString() as I cannot confirm whether this is part of ISO 8601; but it is close enough to other representations, and if ISO 8601 did standardize this, I'm confident it would have this format.

BenMorel commented 1 year ago

Can you please rebase #85?

gnutix commented 1 year ago

Can you please rebase #85?

Sure, first thing tomorrow morning. After it's merged, would you mind doing a release ?

BenMorel commented 1 year ago

@gnutix I'll do what I can :wink: