-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Change Entity Namespace #4577
Change Entity Namespace #4577
Conversation
@paulbalandan i'm done here, would you like to review this? |
What is the benefit of new namespace? next we need to move "Model" classes to Model namespace? The current usage is easier to access (no need deep namespace extends) for mostly db based apps. |
The benefit is not about the namespace, it's about organize system directories/sub-directories |
I'm done here. I would like @MGatner to take final look. |
system/Entity.php
Outdated
} | ||
$this->_cast = $cast; | ||
return $this; | ||
parent::__construct($data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing,, removed :D
4db0ffc
to
d1d6a5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Any final requests or concerns?
should i add Services::entity() in a separated PR? or push it in this PR? or we shouldn't added it anyway? |
Let's hold off on a service for now. I'd like some resolution to our interface discussion before we start adding more services for classes that don't use interfaces. |
Change Entity class namespace
Refactor Entity casts