search

ml-archive / readme

Welcome to Vapor development at Nodes 📖
56 stars 90 forks source link

Sugar: Make timestamps optional #39

Closed steffendsommer closed 7 years ago

steffendsommer commented 7 years ago

See: https://github.com/nodes-vapor/sugar/issues/9

BrettRToomey commented 7 years ago

Timestamps and the soft delete are all opt-in. I argue that, if a model doesn't want timestamps it should conform to Model not NodesModel. What does everyone else think?

steffendsommer commented 7 years ago

From my point of view, I think it depends on how we look at this sugar. Is it something we see as getting into Vapor or is it something we use internally? If it should go into Vapor, then it makes sense to have an opt in/out feature imo. And I do think it makes sense to get this into Vapor at some point (like it was part of Laravel).

@Casperhr thoughts?

BrettRToomey commented 7 years ago

If we're getting this into Vapor the implementation is going to change a lot because I'll have access to fields I didn't have access to while writing these extensions. If that's the only reason for making the timestamps optional then I still argue not inheriting from NodesModel.

@steffendsommer @Casperhr What do you two think?

steffendsommer commented 7 years ago

Good point @BrettRToomey. So the only downside is that, if you want soft-delete but not timestamps, then that wouldn't be possible atm, right? Even though it's an edge case, I still think the effort for supporting this is minimal - what do you think?

steffendsommer commented 7 years ago

Closing since all of this is coming from Vapor 2 now.