Skip to content

Conversation

@jguittard
Copy link
Contributor

Fixes #26

Signed-off-by: Julien Guittard <julien.guittard@me.com>
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.

Thank you for the PR :)

Let's go with

"php-db/phpdb": "^0.1.0"

fix: fixture loader return types in UT

Signed-off-by: Julien Guittard <julien.guittard@me.com>
@jguittard
Copy link
Contributor Author

Let's go!

@tyrsson tyrsson self-assigned this Sep 6, 2025
tyrsson
tyrsson previously approved these changes Sep 6, 2025
@tyrsson
Copy link
Member

tyrsson commented Sep 6, 2025

Meh, sometimes I hate psalm. You will need to reset the baseline for psalm before it will pass :(

See composer.json

"sa-set-baseline": "psalm --set-baseline",

For whatever reason in psalm 6 I have not been able to get --update-baseline to work as expected so just run set baseline twice and it should clear that issue.

Also, if you will retarget this PR at the 0.1.1 branch it should create a merge_up branch for merging into 0.2.0

@tyrsson tyrsson added this to the 0.1.1 milestone Sep 6, 2025
Signed-off-by: Julien Guittard <julien.guittard@me.com>
@jguittard
Copy link
Contributor Author

Haha, psalm can be very picky fore sure, I'd rather revert the fixes I made to FixtureLoader interface and leave the baseline as-is for now

…\FixtureLoader

Signed-off-by: Julien Guittard <julien.guittard@me.com>
@tyrsson
Copy link
Member

tyrsson commented Sep 6, 2025

Well, the situation is that either the typing needs to be resolved (and the baseline reset) or a annotation added to the FixtureLoader. If you can just retarget this to the 0.1.1 branch (it needs fixed as well). Then I will merge it since I will shortly be opening a PR to address a few other issues I will include the typing fixes for 0.2.0. The CI/CD pipeline really is just a sanity check at the moment. We will be VERY strict on that once we reach 1.0 of course. But, this is pre beta code so it should always be considered broken :)

The real issue is that were just now really getting to a point where were ready for PR's. :) There has been a ton of work required just to split the code into the satellite packages and refactor the entry to the lib using factories backed by SM4. My todo list for this weekend already had this change on it but I would much rather merge the work from you rather than just include it in a PR. You did the work :)

Just remember, this code will be influx for at least another month or so while we remove code from phpdb that is moving to satellite packages. There is a scope of work and several tracking issues in the various repo's that I try my best to keep updated... It's just A LOT to keep track of on a daily basis.

@jguittard jguittard changed the base branch from 0.2.x to 0.1.x September 6, 2025 21:13
@tyrsson tyrsson merged commit d62b954 into php-db:0.1.x Sep 6, 2025
7 of 8 checks passed
@jguittard
Copy link
Contributor Author

I am aware about the tons of things that need to be addressed. I'd be happy to help, even though at the very moment the core topics are already handled.
Let me know if you need help!
I fixed the returned issues for PHPCS, but psalm wouldn't reset the baseline locally... :/
PR is set on 0.1.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

Dependency failure

2 participants