Skip to content

Migrate the SQLite code from the main laminas-db repo to this one #3

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

Open
wants to merge 12 commits into
base: 0.0.x
Choose a base branch
from

Conversation

settermjd
Copy link
Contributor

This PR includes @simon-mundy's core work, and some extra clean up after running the QA tools on the code base.

@settermjd settermjd requested review from tyrsson and simon-mundy June 2, 2025 10:26
@settermjd settermjd self-assigned this Jun 2, 2025
These changes, while by no means complete, are a start along the way to
cleaning up the codebase so that it is as simple as possible to perform
static analysis on.
tyrsson
tyrsson previously approved these changes Jun 2, 2025
Copy link
Member

@tyrsson tyrsson left a comment

Choose a reason for hiding this comment

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

There is a considerable amount of code that will be removed once its refactored for factories.

This change, similar to previous ones, contains a series of changes
designed to improve the quality of static analysis with Psalm (and code
readability and maintainability). I don't believe that there are,
effectively, any actual code changes.
This change resets Psalm's baseline, and updates its configuration to
ignore a series of rules that are, currently, not required, as they're
making the transition more challenging. Specifically, they can be
ignored for the time being while the code is cleaned up, after which
they'll be reinstated.
This adds Psalm to the QA job, so that it's included in that task,
ensuring that the code is as maintainable as possible.
@settermjd settermjd marked this pull request as ready for review June 16, 2025 10:43
@settermjd
Copy link
Contributor Author

Sorry about the sheer number of files in this PR.

@settermjd
Copy link
Contributor Author

There is a considerable amount of code that will be removed once its refactored for factories.

Would love to discuss this fully.

@tyrsson
Copy link
Member

tyrsson commented Jun 16, 2025

I'm happy too anytime we're both online.

$pdoDriver = substr($pdoDriver, 3) ?: '';
}
break;
case 'pdodriver':
Copy link
Member

Choose a reason for hiding this comment

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

I missed this in my initial review. I think it would serve us well to remove the one off parameters in favor of using just the 'driver' key. One of my goals is to standardize this to ease documenting all of the changes. I can not confirm this but my hunch is that we find these one offs due to so many different contributors providing code over the years.

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