search

RatkoR / laravel-crate.io

Crate.io driver for Laravel
MIT License
36 stars 12 forks source link

Change timestamp #28

Closed JulianMar closed 5 years ago

JulianMar commented 5 years ago

I changed the default timestamp to a ISO 8601 Date.
With the timestamp all dates were wrong in the database. With the c format it does work.
I changed the way the dateFormat gets resolved aswell. Now it is easier to overwrite the default dateFormat.

RatkoR commented 5 years ago

The first commit (35c5852cc145b756a229b881e005d069cca621a1) is ok. I can cherry-pick it and merge it to master.

The second one (5d375c37c90c087f2f3057f9353dec52ebf71d0e) seems to be a breaking change. I don't think it's wise to push it to users. Maybe all we should do is to do a section in readme and inform users there that c option for date format can be more accurate time representations then timestamps (probably varies case by case and users can stil choose dateFormat as they wish).

What do others think?

JulianMar commented 5 years ago

This would be okay for me aswell!

If I use the default behaviour the time was wrong in crate. Everything showed up as 1970 and a few hours. With the Change to c it did show up correctly. But I still had a problem, because the timestamp were not cast correctly if I retrieved them from the DB.
After changing the dateFormat to Y-m-d\TH:i:s it did work as expected.