codeigniter4
Upsert + Refactor BaseBuilder *Batch() Methods
This PR is a combination of https://github.com/codeigniter4/CodeIgniter4/pull/6367 and https://github.com/codeigniter4/CodeIgniter4/pull/6373 as well as refactoring all the Batch methods.
This is the bigger picture of where those PR's are headed. You can consider this PR or not. Its hard to break things down much smaller because everything is interdependent.
I would suggest creating a special branch for upsert and refactor of Batch methods. Perhaps we could merge this PR there instead. This way we all may work together on it and contribute to it. I welcome any help!
This fixes https://github.com/codeigniter4/CodeIgniter4/issues/5674
Checklist:
- [x] Securely signed commits
- [x] Component(s) with PHPDoc blocks, only if necessary or adds value
- [x] Unit testing, with >80% coverage
- [ ] User guide updated
- [x] Conforms to style guide
This PR is too big (+2,441, −267), and the commit is also too big.
It is not acceptable, because it has multiple changes.
One thing at a time: A pull request should only contain one change. https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#branching
The change in one commit is also so large that it is difficult to understand the code. I believe that merging this would be an obstacle to future maintenance. I hope a PR like this size has more than 200 commits.
So I think this PR is just a reference.
Our PR guideline does not mention about commit messages well now. I put some references for writing good commit messages:
I would suggest creating a special branch for upsert and refactor of Batch methods. Perhaps we could merge this PR there instead.
I can try. A lot of the changes take place simultaneously.
The only way I could properly plan and put it all together was to put it all together at once. There are things shared between upsert and update and there are things shared between upsert and insert. I had to put them all together to normalize these common things. Then refactor the methods that set and execute them.
It would be a lot of piece work to try and do it one by one. I could summarize the major changes though.
I had all the commits at one time but you complained there were too many so I merged them all together.
There are a few things Id like to discuss with what is here. Once those things are figured out then I can rebuild it piece by piece if necessary.
The old code was really screwy. Things were being set in all different places. Now there is one method to set data.. setBatch(). Any data that comes in through the other methods is referred to setBatch()
Create a new branch for this and I will submit it in pieces and explain each piece.
I had all the commits at one time but you complained there were too many so I merged them all together.
I don't recall complaining about too many PR commits, but maybe there was a problem with my English.
I recommend atomic commits. Also, I would like the overall size of a PR to be small.
Its best to review this code by looking at the code and not the "files changed".