Skip to content
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

merge ActiveRelation into ActiveQuery #2497

Merged
merged 5 commits into from
Feb 21, 2014
Merged

Conversation

cebe
Copy link
Member

@cebe cebe commented Feb 20, 2014

allow extending only one class to add scopes, fixes #2146

TODO:

  • adjust guide docs
  • adjust README files of extensions
  • finish work and fix test breaks

allow extending only one class to add scopes, fixes #2146

TODO:

- [ ] adjust guide docs
- [ ] adjust README files of extensions
- [ ] finish work and fix test breaks
@cebe cebe added this to the 2.0 Beta milestone Feb 20, 2014
@qiangxue
Copy link
Member

Great job!

Should we also merge ActiveQueryInterface with ActiveRelationInterface, and ActiveQueryTrait and ActiveRelationTrait? Separating them doesn't seem to have benefit since they are always used as a whole.

@cebe
Copy link
Member Author

cebe commented Feb 20, 2014

They are but it is good to separate the logic imo. This way you see which methods are for which kind of query by just checking from which trait they come for example. It also still possible to create AR with an extra relation class if there is any need.

cebe added a commit that referenced this pull request Feb 21, 2014
WIP merge ActiveRelation into ActiveQuery
@cebe cebe merged commit 5ee50a8 into master Feb 21, 2014
@cebe cebe deleted the merge-query-and-relation2 branch February 21, 2014 18:01
@cebe
Copy link
Member Author

cebe commented Feb 21, 2014

Updated the docs and merged.

@rawtaz
Copy link
Contributor

rawtaz commented Feb 21, 2014

Should the changes in .travis.yml and composer.json really be in there? :o

@cebe
Copy link
Member Author

cebe commented Feb 21, 2014

They are not related to this change but I made them while noticing that the test chain was broken a bit when working local so I fixed it.

@qiangxue
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActiveRelation: support custom Scopes
3 participants