Open dakujem opened 5 years ago
Hello, David. Does this look good to you? I will eventually solve the currently present issues, I hope. I'm in a hurry now though.
Sidenote: I spent unnecessary time to find out how to run the tests so that most of them are not skipped... May I suggest adding a brief info for other devs how to run the tests?
Another sidenote: I was wondering why are the two methods (where
and having
) so inconsistent. Why does having
only accept a string, while where
will happily accept an array of string as well? All the while multiple calls to having
will overwrite the HAVING condition, while multiple calls to where
will add more WHERE conditions. This I find suboptimal, as both HAVING and WHERE are similar in the way the resulting SQL is built.
Does this look good to you?
Thanks, it looks great!
I spent unnecessary time to find out how to run the tests so that most of them are not skipped... May I suggest adding a brief info for other devs how to run the tests?
That would be great, I don't realize these things. Maybe add it to a the file contributing.md
?
I was wondering why are the two methods (where and having) so inconsistent.
Unfortunately, I don't know, I'm not an author. It could be unified, but I don't know how big a BC break would be.
Adds
Selection::havingOr
method analogously to theSelection::whereOr
, refactoring the common logic into a separate protected method (Selection::paramsOr
).closes #240