Skip to content

Add countMatching()#15

Open
holtkamp wants to merge 1 commit into
rikbruil:masterfrom
holtkamp:patch-2
Open

Add countMatching()#15
holtkamp wants to merge 1 commit into
rikbruil:masterfrom
holtkamp:patch-2

Conversation

@holtkamp

@holtkamp holtkamp commented Nov 5, 2016

Copy link
Copy Markdown
Contributor

This allows simple counting functionality which return a single scalar result:

return (int) $this->countMatching($specification)->getSingleScalarResult();

This allows simple counting functionality which return a single scalar result:


```php
return (int) $this->countMatching($specification)->getSingleScalarResult();
```
@coveralls

coveralls commented Nov 5, 2016

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-3.8%) to 95.804% when pulling b4e5507 on holtkamp:patch-2 into 4a53c7a on rikbruil:master.

@holtkamp

holtkamp commented Nov 7, 2016

Copy link
Copy Markdown
Contributor Author

@rikbruil sorry for the lack of any tests. I had a look at existing tests / specifications, but this is quite complex for me at this point... No idea on how the added functionality should be tested...

Instead of a complete SELECT e FROM entity only a SELECT count(e) FROM entity is generated... hope the idea is clear.

@rikbruil

rikbruil commented Jan 5, 2017

Copy link
Copy Markdown
Owner

I will take a look at making the coverage better

@holtkamp

Copy link
Copy Markdown
Contributor Author

@rikbruil hey Rik, I am cleaning up "custom" code and I encountered this pending PR, would/could you consider merging it?

@holtkamp

Copy link
Copy Markdown
Contributor Author

Small reminder here as well

@holtkamp holtkamp requested a review from rikbruil February 17, 2020 14:36
@holtkamp

holtkamp commented Feb 1, 2021

Copy link
Copy Markdown
Contributor Author

Hi @rikbruil, are you still using this library? Still very powerful I would say...

@rikbruil

Copy link
Copy Markdown
Owner

@holtkamp Hey Menno, sorry for the lack of responses here... I keep reading the notifications on my phone and then later forget them when I'm behind a keyboard again.

Yeah my employer is still using this in production, although personally I don't work that often with this library anymore since I'm rarely touching Doctrine lately. That's also why I've been a bit hesitant merging changes lately, since I'm not personally using it and can't directly see the impact of a merge on our production environment (unless the added code is covered well with tests).

I remember I looked into the coverage for this PR before, but I don't remember having a fix for it. I will take another look in the evening and get back to you here. Also, if you are still using this library a lot perhaps I could consider making you a maintainer as well? (if that's possible, I'm not sure on this).

Because I would hate for my late responses to block you if you are trying to use this library in production as well.

@holtkamp

Copy link
Copy Markdown
Contributor Author

@rikbruil no problemo and I fully understand.

Yes I use the library a lot and love the charm of it. So I am flattered for being asked as a maintainer. Stuff like static analysis and using GitHub Actions to trigger them would be nice to have and things I might be able to contribute....

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants