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

Support and require laminas-servicemanager v4.0 #25

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

boesing
Copy link
Member

@boesing boesing commented Jul 2, 2023

Q A
BC Break yes
New Feature yes

Description

This provides support for laminas-servicemanager v4.0

@boesing boesing added this to the 3.0.0 milestone Jul 2, 2023
@boesing boesing changed the base branch from 2.15.x to 3.0.x July 2, 2023 16:45
@boesing boesing force-pushed the feature/servicemanager-4.0 branch from a74f4c8 to 6fb97f7 Compare July 2, 2023 16:46
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the feature/servicemanager-4.0 branch from 6fb97f7 to db5be2f Compare July 2, 2023 16:47
@@ -31,19 +31,18 @@
}
},
"require": {
"php": "~8.0.0 || ~8.1.0 || ~8.2.0",
"php": "~8.1.0 || ~8.2.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

laminas-servicemanager dropped support for PHP 8.0 and thus we can drop it here as well.

"laminas/laminas-json": "^3.1",
"laminas/laminas-servicemanager": "^4.0.0-rc2",
Copy link
Member

Choose a reason for hiding this comment

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

title says "add support", but implementation adds it as a requirement (wasn't one before, BTW).

Not sure what the right direction is here 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIR we decided on stating explicit requirements in components to avoid surprises in upstream projects.

The AdapterPluginManager and the Serializer abstract class are totally depending on the AdapterPluginManager. So the only things which are not servicemanager related are the concrete implementations.

Not sure if we should extract the plugin manager specific code to a satellite instead but having 60+% of this component being dangling until composer requirements are fulfilled feels weird to me.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me 👍

@boesing boesing changed the title Add support for laminas-servicemanager v4.0 Support and require laminas-servicemanager v4.0 Jul 3, 2023
@boesing boesing merged commit 800c6c0 into laminas:3.0.x Jul 7, 2023
@boesing boesing deleted the feature/servicemanager-4.0 branch July 7, 2023 21:49
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.

2 participants