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

Narrow array type for services in ServiceManager configuration #213

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

rieschl
Copy link
Contributor

@rieschl rieschl commented Oct 14, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

This PR adds a more specific array shape for the services key in ServiceManagerConfiguration.

fixes #212

@boesing boesing linked an issue Oct 16, 2023 that may be closed by this pull request
@boesing boesing changed the title QA: Use a more specific array shape in services configuration of ServiceManager Narrow array type for services in ServiceManager configuration Oct 16, 2023
@boesing boesing added this to the 3.22.1 milestone Oct 16, 2023
@@ -89,7 +89,7 @@
* initializers?: InitializersConfiguration,
* invokables?: array<string,string>,
* lazy_services?: LazyServicesConfiguration,
* services?: array<string,object|array>,
* services?: array<string,object|array<array-key,mixed>>,
Copy link
Member

Choose a reason for hiding this comment

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

As requested in #212, please use array<mixed> instead.

Copy link
Member

@boesing boesing Oct 16, 2023

Choose a reason for hiding this comment

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

@laminas/technical-steering-committee should we enforce this kind of array notation via our codestandard and if so, are there existing rules for that? That would prevent us from having these kind of "issues" with incompatibilities between psalm/phpstan in the future.

@rieschl rieschl force-pushed the qa/service-manager-array-shape branch from 780fab7 to 10ceb4a Compare October 16, 2023 19:06
@rieschl rieschl requested a review from boesing October 16, 2023 19:06
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM, will see when I find some time to prepare a release

nvm, is just a patch. going to release now.

@boesing boesing merged commit de98d29 into laminas:3.22.x Oct 24, 2023
12 checks passed
@rieschl
Copy link
Contributor Author

rieschl commented Oct 24, 2023

LGTM, will see when I find some time to prepare a release

nvm, is just a patch. going to release now.

thanks!

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.

Array shape of ServiceManager config contains unspecific array
2 participants