-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
!!! TASK: Raise doctrine/dbal version constraint #5161
Conversation
71e54b3
to
d4b37b3
Compare
No longer because we have upmerged the double compat from 8.4 |
Added a commit that should be dropped later which should turn the CI green to see as a real e2e check if we missed something in flow. |
6b971c4
to
80d2b53
Compare
Neos.ContentRepository.LegacyNodeMigration/Classes/Command/CrCommandController.php
Outdated
Show resolved
Hide resolved
Neos.ContentRepository.LegacyNodeMigration/Classes/Command/CrCommandController.php
Outdated
Show resolved
Hide resolved
Neos.ContentRepository.Core/Classes/Infrastructure/DbalCheckpointStorage.php
Outdated
Show resolved
Hide resolved
I call this fine for now, see comments regarding the linting problem. |
80d2b53
to
9ace319
Compare
Adjust to changes in DBAL API
9ace319
to
7e7245d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good by reading, and given the tests pass, this should be fine.
Just make sure to drop the CI hack before merge!
Neos.ContentRepository.Core/Classes/Infrastructure/DbalCheckpointStorage.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out locally (by lazily creating a new core branch :D)
And Neos seems to boot up.
Also run the Neos Ui e2e tests and they pass.
Thank you for this <3
"neos/neos-development-collection": "dev-task/temporary-core-branch-for-5161 as 9.0",
"neos/flow-development-collection": "dev-dependabot/composer/doctrine/dbal-tw-3.2 as 9.0",
also we require https://github.com/neos/eventstore-doctrineadapter/releases/tag/2.0.0 and thus you have to remove "neos/eventstore-doctrineadapter": "^1"
or raise it to 2
in your dev dist
The only thing i dindt test gotta be the new media module.
Also i have no idea how we deal with outdated import statements, ill push a commit to adjust use Doctrine\DBAL\DBALException
but the scary thing is CI didnt catch it and i dont know if phpstan can.
edit: phpstan works in code that we check, but we dont check everything yet ^^
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php
Outdated
Show resolved
Hide resolved
Neos.ContentRepository.Core/Classes/Infrastructure/DbalCheckpointStorage.php
Outdated
Show resolved
Hide resolved
7e7245d
to
d01548b
Compare
ähhh @mhsdesign you forced pushed and it's broken now? |
i dropped my CI hack commit as i think this is ready to merge |
Right, flow too? then lets goooooo |
Alright, I would carefully say this is fine now, whatever might still need adapting can be done as a follow up. |
Lol the total diff has become kinda unreviewable with all the casing changes 😅 |
Well it's a PITA with that exception, I tried to just unify it to DBALException now, instead of doing it one by one because meh. |
I think nothing in here is raelly a big change apart from the iterable result thing in the media repos... |
Neos adjustments for neos/flow-development-collection#2637
Related neos/eventstore-doctrineadapter#17
This may be breaking if you use the iteratable methods of the media Repositories, as those work slightly different now.
You can directly iterate the result of the query operation instead of going back to the iterate method.