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.

Sign up for GitHub

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

Jump to bottom

feat: Add Matching operation and implements Selectable from Doctrine. #115

Merged
merged 10 commits into from
Jul 5, 2021
Merged

feat: Add Matching operation and implements Selectable from Doctrine. #115

merged 10 commits into from
Jul 5, 2021

Conversation

drupol
Copy link
Collaborator

@drupol drupol commented Jun 24, 2021 •
edited

This PR

  • Implements Collection::matching(Criteria $criteria) operation
  • Has tests
  • Update documentation
  • Fix static analysis <- Do we need them?

To investigate:

@drupol drupol force-pushed the feature-works-with-doctrine-criteria branch 3 times, most recently from cfef956 to 8783b22 Compare June 24, 2021 08:05
@AlexandruGG
Copy link
Collaborator

tag me when this is ready, I'll want to review :)

@drupol drupol force-pushed the feature-works-with-doctrine-criteria branch from 8783b22 to b84c761 Compare June 24, 2021 18:39
@drupol drupol marked this pull request as ready for review June 24, 2021 19:31
@drupol drupol requested a review from AlexandruGG June 24, 2021 19:31
@drupol drupol force-pushed the feature-works-with-doctrine-criteria branch 4 times, most recently from 424a3aa to aba42a9 Compare July 2, 2021 09:37
@drupol drupol force-pushed the feature-works-with-doctrine-criteria branch from aba42a9 to 1c4428f Compare July 3, 2021 20:30
@drupol drupol added the enhancement New feature or request label Jul 3, 2021
AlexandruGG
AlexandruGG reviewed Jul 3, 2021
View reviewed changes
docs/pages/api.rst Outdated Show resolved Hide resolved
src/Contract/Operation/Matchingable.php Outdated Show resolved Hide resolved
src/Contract/Operation/Matchingable.php Outdated Show resolved Hide resolved
phpstan-baseline.neon Outdated Show resolved Hide resolved
drupol and others added 2 commits July 3, 2021 22:42
@drupol @AlexandruGG
Update docs/pages/api.rst
8b7d57e 
Co-authored-by: Alex Gidei <34811569+AlexandruGG@users.noreply.github.com>
@drupol @AlexandruGG
Update src/Contract/Operation/Matchingable.php
6f08655 
Co-authored-by: Alex Gidei <34811569+AlexandruGG@users.noreply.github.com>
@AlexandruGG
Copy link
Collaborator

Should we detach each row from Doctrine? (https://www.doctrine-project.org/projects/doctrine-orm/en/latest/reference/batch-processing.html#iterating-large-results-for-data-processing)

Regarding this, I think it could be implemented in a follow-up PR with a callback provided by the user. I don't think it should be done "by default", and also we don't have access to the entity manager, right? The user would have to provide that

AlexandruGG
AlexandruGG approved these changes Jul 5, 2021
View reviewed changes
Copy link
Collaborator

@AlexandruGG AlexandruGG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this 💯

@drupol
Copy link
Collaborator Author

drupol commented Jul 5, 2021

Thanks!

@drupol drupol merged commit 3a426cd into master Jul 5, 2021
@drupol drupol deleted the feature-works-with-doctrine-criteria branch July 5, 2021 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexandruGG AlexandruGG AlexandruGG approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@drupol @AlexandruGG