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

Add optional parameter to like(), or_like(), not_like() and or_not_like() methods. #5643

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

nalakapws
Copy link

This PR address the 2nd feature request from this #5641 issue.

…) and or_not_like() methods. which can be used to choose whether use those methods with WHERE clause or HAVING clause.
@narfbg
Copy link
Contributor

narfbg commented Dec 12, 2018

This is parameter creep ... It may look inconsistent, but a better option would be to just have having_like(), having_not_like(), etc.

And you must follow the styleguide.

@narfbg
Copy link
Contributor

narfbg commented Dec 17, 2018

You're still ignoring the styleguide ...

@nalakapws
Copy link
Author

Is there anything I have to do in order to make this PR get merged?

@narfbg
Copy link
Contributor

narfbg commented Jan 4, 2019

Yes, there are still some spaces left in on lines 1100, 1171 (shown in diff with darker green).

And there's more significant stuff code-wise, but it will probably result in a lot of back and forth if I asked you to do all of it, so I can make those changes myself after merging. It's decent enough as it is, I just want less repetition.

…ng_group_start(), or_not_having_group_start() and having_group_end() methods for HAVING clause.
Copy link

@error56 error56 left a comment

Choose a reason for hiding this comment

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

All the code doesn't match PSR-2 rules

system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
system/database/DB_query_builder.php Show resolved Hide resolved
@narfbg
Copy link
Contributor

narfbg commented Feb 28, 2019

Yes, all of CodeIgniter doesn't match PSR-2 rules. We have our own styleguide, referenced in the contributing guide, which you should read before reviewing others' work.

* @param bool $escape
* @return CI_DB_query_builder
*/
public function not_having_like($field, $match = '', $side = 'both', $escape = NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be having_not_like instead.
Same thing applies to all other methods below - not should always be after having in the name.

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.

3 participants