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
Split combined setters/getters for cleaner API. #166
Conversation
Codecov Report
@@ Coverage Diff @@
## master #166 +/- ##
============================================
+ Coverage 99.68% 99.69% +<.01%
- Complexity 138 140 +2
============================================
Files 11 11
Lines 321 325 +4
============================================
+ Hits 320 324 +4
Misses 1 1
Continue to review full report at Codecov.
|
@@ -65,12 +65,12 @@ public function repository() | |||
/** | |||
* Return Table | |||
* | |||
* @return \Cake\ORM\Table Table Instance | |||
* @return \Cake\Datasource\RepositoryInterface Repository Instance |
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.
This does change the return and it's not BC per-se, but I'm going to merge this in as it's the proper way to do things.
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 fixes only the doc block (semantic) to what the non-deprecated method already returns.
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.
Yep thats why I'm fine with merging as is.
Continue #165
The different return types of useCollection() vs getCollection is especially important to clean up here.