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

initial signed commit of code changes to add ResultInterface::getNumRows #4048

Closed
wants to merge 1 commit into from
Closed

Conversation

sneakyimp
Copy link
Contributor

And also to the various DBMS-specific Result classes. Added unit tests/system/Database/Live/GetNumRowsTest.php for this newly added method. Added an entry in the user docs for getNumRows method.

Description
Per Lonnie Ezell's message in #109, I have attempted my first CodeIgniter fork/branch/pull request here. It adds a getNumRows method to the ResultInterface and the various DBMS classes that extend it. This pull request supercedes a previous messy one (for which I apologize).

Checklist:

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

…abase/ResultInterface and the various DBMS-specific Result classes. Added unit tests/system/Database/Live/GetNumRowsTest.php for this newly added method. Added an entry in the user docs for getNumRows method.
@sneakyimp
Copy link
Contributor Author

Once again, I've made some mistakes. Closing this request, too. Will run phpstan and see if i get better results.

@sneakyimp sneakyimp closed this Jan 1, 2021
@sneakyimp sneakyimp deleted the num-rows-signed branch January 1, 2021 01:47
@kenjis
Copy link
Member

kenjis commented Jan 1, 2021

@sneakyimp You didn't make any mistakes. This PR is better than previous one.
You just have to fix MockResult.

@sneakyimp
Copy link
Contributor Author

@kenjis it's failing all the tests. I want it to be clean. I also left out the user documentation change. I've got one more I think will do it. Will submit momentarily.

@kenjis
Copy link
Member

kenjis commented Jan 1, 2021

@sneakyimp See #4047 (comment)

@sneakyimp
Copy link
Contributor Author

@sneakyimp See #4047 (comment)

I have addressed that and have submitted a new PR (see #4049 ). Rector doesn't like it (the SQLite3 implementation does not support the function and just throws an exception) but I think it's worth a look.

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.

2 participants