-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[BC Break] Initial repository cleanup #6873
[BC Break] Initial repository cleanup #6873
Conversation
michalmarcinkowski
commented
Nov 26, 2016
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | yes |
Related tickets | |
License | MIT |
3457af8
to
46e7d92
Compare
46e7d92
to
ddb3f3c
Compare
->getResult() | ||
; | ||
} | ||
} |
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.
Why removing this? You pretty much always need zone with its members.
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 method is only used in a Behat context and I'm not sure you will ever use it in your application, because it's more likely that you will use code
for querying, zone name is not unique.
Using "default" joins is a separate topic, if we want to introduce it we should do it for all entities.
; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createListQueryBuilder($locale) | ||
public function findByName($name, $locale) |
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.
findOneByName?
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.
The name is not unique, so we can have more than 1 object returned by this method.
@@ -24,7 +24,6 @@ public function createListQueryBuilder($locale) | |||
{ | |||
return $this | |||
->createQueryBuilder('o') | |||
->addSelect('translation') |
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.
I am pretty sure this will cause to lazy load translation contents anyway, as they will not be part of the select clause.
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.
I will check
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.
You're right! There are many methods without selects, we will have to add them...
Thanks Michał! |
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup
…leanup [BC Break] Initial repository cleanup