Conversation
|
I believe it's best to add for v2. |
|
Ok. I can rebase this changes to 2.0 branch later. Should I drop BC aliases then? |
|
Yes, we can BC them for v2. |
|
I think this PR can be ok for v1.x |
|
You can approve it then :) |
👍 |
tntrex
force-pushed
the
65-rename-constrain-to-scope
branch
2 times, most recently
from
1617be5 to
0b2566f
Compare
tntrex
force-pushed
the
65-rename-constrain-to-scope
branch
from
0b2566f to
7e0f3a9
Compare
|
Ping @roxblnfk |
|
If @hustlahusky will do not force push changes with overwriting previous commits then i ready to review ;) |
|
Also not all tests passed |
Yeah. I finished with changes for now :)
Tests are ok. Only check failed is code style caused by warnings in BC aliases files. It needs to be ignored, but I don't know how to do that |
roxblnfk
left a comment
roxblnfk
left a comment
There was a problem hiding this comment.
I think we need to add tests that check:
- old methods are work
- old interfaces (classes) are work
- old options and constants work too
| * @return SourceInterface | ||
| */ | ||
| public function withConstrain(?ConstrainInterface $constrain): SourceInterface; | ||
| public function withScope(?ScopeInterface $scope): SourceInterface; |
There was a problem hiding this comment.
I understand that this is an internal API, but adding new methods to the interface is a major change. @wolfy-j , what do you think about that?
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Cycle\ORM\Select\Scope; |
There was a problem hiding this comment.
This class can placed in the Cycle\ORM\Selectnamespace similar to the \Cycle\ORM\Select\ConstrainInterface.
Why do you think it is worth allocating a separate directory for it?
There was a problem hiding this comment.
I have plans to add some more scope options in future for #24, that can be helpful for implementing extra lazy collection behavior like in doctrine
I think you can move deprecated stubs in the |
|
Split small improvements that can be merged to v1 in #191 |
Be careful, because now branch 2.0 is actively changing. |
3 participants
Closes #65