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

Fix not compatable with base model in ci 4.5 #52

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

Conversation

seunex17
Copy link

After upgrade to Codeigniter version 4.5 it broken while using the ModelTrait look like Codeigniter changed thier findAll() method.

i update $limit = 0 to $limit = null

CI4 now use null as default value to limit instead of 0

Screenshot 2024-04-24 at 7 42 38 AM

@damian-romanowski
Copy link

@MGatner Can this please be merged in. Struggling with quite a few projects that require this change..

@MGatner
Copy link
Collaborator

MGatner commented May 23, 2024

Has anyone tested if this is backwards-compatible? We probably need to bump the CodeIgniter framework minimum version alongside this change.

@damian-romanowski
Copy link

Has anyone tested if this is backwards-compatible? We probably need to bump the CodeIgniter framework minimum version alongside this change.

I only encountered this after updating to v4.5.0 so yeah, I believe this should bump the min ver to ^4.5.

@maldini4869
Copy link

any updates with the pr?

@MGatner
Copy link
Collaborator

MGatner commented Jun 6, 2024

I don't use this library myself anymore except in locked legacy projects. Honestly the experience of working with an ORM-ish in CI4 pushed me in the opposite direction, and I now favor a very bespoke Repository approach.

I am happy to maintain this library but it's harder to do things like backwards-compatibility testing. For this PR specifically I think we need to lock the framework dependency so we don't mess up older projects. If @seunex17 is not available to do that today/tomorrow then I can handle it post-merge.

@seunex17
Copy link
Author

seunex17 commented Jun 6, 2024

Okay! @MGatner am going to address this in the next hour.

@seunex17
Copy link
Author

seunex17 commented Jun 7, 2024

@MGatner please have a look and let me know it that okay to move on.

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.

None yet

4 participants