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

Allow psr/container 2.0.* #175

Closed
wants to merge 2 commits into from
Closed

Conversation

tobilektri
Copy link

@tobilektri tobilektri commented Feb 15, 2023

Should allow psr/container 2.0.2 - not only 1.1.2

Changes:
php-fig/container@2.0.2...1.1.2

Can close after merge: #151

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

Description

Should allow psr/container 2.0.2 - not only 1.1.2

Changes:
php-fig/container@2.0.2...1.1.2

Signed-off-by: Tobias Meindl <[email protected]>
Signed-off-by: Tobias Meindl <[email protected]>
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Sadly not as simple as it sounds, as this represents a major BC break for downstream.

@Ocramius
Copy link
Member

Duplicate of #151

@Ocramius Ocramius marked this as a duplicate of #151 Feb 15, 2023
@Ocramius Ocramius closed this Feb 15, 2023
@Ocramius Ocramius added the Duplicate This issue or pull request already exists label Feb 15, 2023
@tobilektri
Copy link
Author

@Ocramius could you please explain why adding a new allowed version is a "major BC break for downstream" - the change in the psr/container is not a major BC in my opinion.
What has to be fixed to allow the ^2.0?

@Ocramius
Copy link
Member

This snippet makes it more clear: https://3v4l.org/DfZYG

<?php

class ServiceManager {
    function has($name): bool { return true; } 
}

final class UserlandContainer extends ServiceManager
{
    function has($name) { return true; } 
}
Fatal error: Declaration of UserlandContainer::has($name) must be compatible with ServiceManager::has($name): bool in /in/DfZYG on line 9

Process exited with code 255.

@tobilektri
Copy link
Author

tobilektri commented Feb 16, 2023

So this change will only take place with the (next?) Major Version release of laminas-servicemanager?

Edit: Thanks for the example! :)

@froschdesign
Copy link
Member

So this change will only take place with the (next?) Major Version release of laminas-servicemanager?

Correct. See also the related milestone for the next major version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break Duplicate This issue or pull request already exists Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants