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

[Proposal] Use Carbon instead of DateTime in Eloquent #999

Closed
kapv89 opened this Issue Apr 18, 2013 · 23 comments

Comments

Projects
None yet
7 participants
@kapv89
Contributor

kapv89 commented Apr 18, 2013

This won't change the api at all, and offer us devs a much more powerful object to work with dates

@agentphoenix

This comment has been minimized.

Show comment
Hide comment

👍

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Apr 19, 2013

Member

I'm pretty much OK with this.

Member

taylorotwell commented Apr 19, 2013

I'm pretty much OK with this.

@robclancy

This comment has been minimized.

Show comment
Hide comment
@robclancy

robclancy Apr 19, 2013

Contributor

Yay I was hoping you would like to use it. Would be cool if you add an alias to skeleton though :p Since Carbon has nothing to do with dates at all.. Why couldn't he name it something with the word date or time :/

Contributor

robclancy commented Apr 19, 2013

Yay I was hoping you would like to use it. Would be cool if you add an alias to skeleton though :p Since Carbon has nothing to do with dates at all.. Why couldn't he name it something with the word date or time :/

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Apr 19, 2013

Member

I think he is referring to "carbon dating", which is used to date old
artifacts, etc.

Member

taylorotwell commented Apr 19, 2013

I think he is referring to "carbon dating", which is used to date old
artifacts, etc.

@kapv89

This comment has been minimized.

Show comment
Hide comment
@kapv89

kapv89 Apr 19, 2013

Contributor

Ummm ... carbon-dating ?
On Apr 19, 2013 7:24 PM, "Robert Clancy (Robbo)" notifications@github.com
wrote:

Yay I was hoping you would like to use it. Would be cool if you add an
alias to skeleton though :p Since Carbon has nothing to do with dates at
all.. Why couldn't he name it something with the word date or time :/


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/999#issuecomment-16654140
.

Contributor

kapv89 commented Apr 19, 2013

Ummm ... carbon-dating ?
On Apr 19, 2013 7:24 PM, "Robert Clancy (Robbo)" notifications@github.com
wrote:

Yay I was hoping you would like to use it. Would be cool if you add an
alias to skeleton though :p Since Carbon has nothing to do with dates at
all.. Why couldn't he name it something with the word date or time :/


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/999#issuecomment-16654140
.

@robclancy

This comment has been minimized.

Show comment
Hide comment
@robclancy

robclancy Apr 19, 2013

Contributor

So we work with old artifacts? Carbon is a chemical element. It's just annoying with things like this having abstract names compared to other languages where things just make sense.

Contributor

robclancy commented Apr 19, 2013

So we work with old artifacts? Carbon is a chemical element. It's just annoying with things like this having abstract names compared to other languages where things just make sense.

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Apr 19, 2013

Member

Take it up with them :) ... I think it's a cool name... we gotta have
some fun here

Member

taylorotwell commented Apr 19, 2013

Take it up with them :) ... I think it's a cool name... we gotta have
some fun here

@robclancy

This comment has been minimized.

Show comment
Hide comment
@robclancy

robclancy Apr 19, 2013

Contributor

I don't care that much and if I did take it up with them they would just laugh at me :p You gonna add this in today or wait until you have a chance to change all the DateTime calls? I want to use it now so just wondering if I should add it to composer.json myself now.

Contributor

robclancy commented Apr 19, 2013

I don't care that much and if I did take it up with them they would just laugh at me :p You gonna add this in today or wait until you have a chance to change all the DateTime calls? I want to use it now so just wondering if I should add it to composer.json myself now.

@jasonlewis

This comment has been minimized.

Show comment
Hide comment
@jasonlewis

jasonlewis Apr 19, 2013

Contributor

Or... Expressive Date! 😀

Contributor

jasonlewis commented Apr 19, 2013

Or... Expressive Date! 😀

@robclancy

This comment has been minimized.

Show comment
Hide comment
@robclancy

robclancy Apr 19, 2013

Contributor

that's too #swaga

Contributor

robclancy commented Apr 19, 2013

that's too #swaga

@jasonlewis

This comment has been minimized.

Show comment
Hide comment
@jasonlewis

jasonlewis Apr 19, 2013

Contributor

But Robbo it has date in its name!

Contributor

jasonlewis commented Apr 19, 2013

But Robbo it has date in its name!

@robclancy

This comment has been minimized.

Show comment
Hide comment
@robclancy

robclancy Apr 19, 2013

Contributor

I know, it's epic!

Contributor

robclancy commented Apr 19, 2013

I know, it's epic!

@kapv89

This comment has been minimized.

Show comment
Hide comment
@kapv89

kapv89 Apr 19, 2013

Contributor

@jasonlewis umm ... since you made it, why did you make it when carbon exists :P ? (i'm looking at the docs of both of them atm)

Contributor

kapv89 commented Apr 19, 2013

@jasonlewis umm ... since you made it, why did you make it when carbon exists :P ? (i'm looking at the docs of both of them atm)

@jasonlewis

This comment has been minimized.

Show comment
Hide comment
@jasonlewis

jasonlewis Apr 19, 2013

Contributor

Hm, can't remember the reason now. It was a while ago. There was a reason though. 😄

Contributor

jasonlewis commented Apr 19, 2013

Hm, can't remember the reason now. It was a while ago. There was a reason though. 😄

@kapv89

This comment has been minimized.

Show comment
Hide comment
@kapv89

kapv89 Apr 19, 2013

Contributor

hehe ... both of them are very similar, though carbon offers some more stuff, so gonna make a pull with Carbon :)

Contributor

kapv89 commented Apr 19, 2013

hehe ... both of them are very similar, though carbon offers some more stuff, so gonna make a pull with Carbon :)

@jasonlewis

This comment has been minimized.

Show comment
Hide comment
@jasonlewis

jasonlewis Apr 19, 2013

Contributor

D'aw. Heartbroken!

Curious, what more does it offer?

Contributor

jasonlewis commented Apr 19, 2013

D'aw. Heartbroken!

Curious, what more does it offer?

@kapv89

This comment has been minimized.

Show comment
Hide comment
@kapv89

kapv89 Apr 19, 2013

Contributor

aww ... it has more helpers, tbh, nothing that can't be added quickly
https://github.com/briannesbitt/Carbon#comparison
https://github.com/briannesbitt/Carbon#common-formats

stuff like this:

$dt = Carbon::now();
echo $dt->diffInYears($dt->copy()->addYear());

i had also put in fluent setters, but then read your lib's code, and saw it has those too ...

Contributor

kapv89 commented Apr 19, 2013

aww ... it has more helpers, tbh, nothing that can't be added quickly
https://github.com/briannesbitt/Carbon#comparison
https://github.com/briannesbitt/Carbon#common-formats

stuff like this:

$dt = Carbon::now();
echo $dt->diffInYears($dt->copy()->addYear());

i had also put in fluent setters, but then read your lib's code, and saw it has those too ...

@jasonlewis

This comment has been minimized.

Show comment
Hide comment
@jasonlewis

jasonlewis Apr 19, 2013

Contributor

Ah yeah, all good all good. Was just curious.

Contributor

jasonlewis commented Apr 19, 2013

Ah yeah, all good all good. Was just curious.

@kapv89

This comment has been minimized.

Show comment
Hide comment
@kapv89

kapv89 Apr 19, 2013

Contributor

:)

Contributor

kapv89 commented Apr 19, 2013

:)

@briannesbitt

This comment has been minimized.

Show comment
Hide comment
@agentphoenix

This comment has been minimized.

Show comment
Hide comment
@agentphoenix

agentphoenix Apr 24, 2013

I noticed that there are still some places in the Eloquent model that use DateTime so when I try to pass in a Carbon date object, an exception gets thrown because Eloquent is expecting DateTime not Carbon.

One place in particular is fromDateTime which is type casting its parameter to DateTime. Not sure if this shouldn't be casting the type or if the type should be changed to Carbon so I left it to you guys to sort that out instead of going down the wrong path.

I noticed that there are still some places in the Eloquent model that use DateTime so when I try to pass in a Carbon date object, an exception gets thrown because Eloquent is expecting DateTime not Carbon.

One place in particular is fromDateTime which is type casting its parameter to DateTime. Not sure if this shouldn't be casting the type or if the type should be changed to Carbon so I left it to you guys to sort that out instead of going down the wrong path.

@Anahkiasen

This comment has been minimized.

Show comment
Hide comment
@Anahkiasen

Anahkiasen Apr 24, 2013

Contributor

Shouldn't matter, Carbon extends DateTime so it matches the class hint.

Contributor

Anahkiasen commented Apr 24, 2013

Shouldn't matter, Carbon extends DateTime so it matches the class hint.

@agentphoenix

This comment has been minimized.

Show comment
Hide comment
@agentphoenix

agentphoenix Apr 24, 2013

Huh. Works now, but a few minutes ago it was throwing an exception and talking about what I was passing in (a Carbon object not matching the type). Chalk it up to one of those mystery errors. Thanks for the quick response @Anahkiasen!

Huh. Works now, but a few minutes ago it was throwing an exception and talking about what I was passing in (a Carbon object not matching the type). Chalk it up to one of those mystery errors. Thanks for the quick response @Anahkiasen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment