-
-
Notifications
You must be signed in to change notification settings - Fork 32
Split ActiveRecord model #412
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
base: master
Are you sure you want to change the base?
Conversation
Tigrov
commented
Apr 21, 2025
Q | A |
---|---|
Is bugfix? | ❌ |
New feature? | ✔️ |
Breaks BC? | ✔️ |
Fixed issues | #388, #358 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #412 +/- ##
============================================
- Coverage 88.65% 88.40% -0.25%
- Complexity 567 571 +4
============================================
Files 16 18 +2
Lines 1357 1371 +14
============================================
+ Hits 1203 1212 +9
- Misses 154 159 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
use Yiisoft\Db\Exception\InvalidArgumentException; | ||
use Yiisoft\Db\Exception\InvalidConfigException; | ||
|
||
interface ActiveRecordModelInterface |
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.
Need description of the interface. What is it for?
From what I see now:
- It holds DB connection.
- It holds metadata such as table name.
- It holds data in terms of property-value pairs.
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.
Overall, I don't see a clear separation/purpose between ActiveRecordModelInterface
and ActiveRecordInterface
. They are still very much inter-dependent, but moved into two interfaces.
* | ||
* @return bool Whether the two active records refer to the same row in the same database table. | ||
*/ | ||
public function equals(self $record): bool; | ||
public function equals(ActiveRecordModelInterface $record): bool; |
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.
It doesn't look correct. Either the method should be moved to ActiveRecordModelInterface
or ActiveRecordInterface
should be used here.
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.
ActiveRecordInterface
should be here and one more method modelEquals(ActiveRecordModelInterface $model)
should be added.
use Yiisoft\Db\Constant\ColumnType; | ||
use Yiisoft\Db\Exception\Exception; | ||
use Yiisoft\Db\Exception\InvalidArgumentException; | ||
use Yiisoft\Db\Exception\InvalidConfigException; | ||
use Yiisoft\Db\Exception\NotSupportedException; | ||
|
||
/** | ||
* This interface is used by {@see ActiveRecordModelInterface}. |
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.
Need a proper description about what is the purpose of this interface and what's the relation with ActiveRecordModelInterface
.