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

testMode() method for BaseBuilder #2269

Merged
merged 2 commits into from
Sep 26, 2019

Conversation

michalsn
Copy link
Member

Description
This PR introduces a new testMode variable that is meant to be used in the testing environment. It replaces all occurrences of $returnSQL, $test and $testing - reducing the number of parameters in a couple of methods.

Important
This should be considered as a BC change since some parameters from methods are removed. Those parameters weren't well documented but there is still a chance that somebody was using them.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@MGatner
Copy link
Member

MGatner commented Sep 26, 2019

I'm super excited to see this happening!

@jim-parry
Copy link
Contributor

@MGatner @lonnieezell Is this something we want in RC.2? I figure it's now or after 4.0.0, given the BC

@lonnieezell
Copy link
Member

I'm all for it happing now.

@jim-parry
Copy link
Contributor

Lookihng good, and simpler :)
Is this ready to go?

@jim-parry jim-parry merged commit 843d46b into codeigniter4:develop Sep 26, 2019
@tangix
Copy link
Contributor

tangix commented Sep 27, 2019

My concern with this is that it won't be possible to (easily) return the actual SQL to do last-minute manipulation (for example inserting stuff that is not in the QB natively) but maybe this is OK?
I use the returned SQL string to insert SQL_CALC_FOUND_ROWS and ON DUPLICATE KEY UPDATE for example.

@michalsn
Copy link
Member Author

@tangix
Copy link
Contributor

tangix commented Sep 27, 2019

@tangix we have a bunch of methods from the getCompiled* family at our disposal.

Yeah, I guess.

@michalsn michalsn deleted the feature/qb_testMode branch July 16, 2020 13:29
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.

5 participants