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

Deprecated SessionIdentifierAwareInterface for removal in v2.0 #61

Merged
merged 3 commits into from
Aug 16, 2024

Conversation

alexmerlin
Copy link
Member

Description

As requested in #59 this commit deprecates SessionIdentifierAwareInterface and moves its getId method to SessionInterface.

@alexmerlin
Copy link
Member Author

Hey @gsteel, thanks for jumping on this one.

Could you please explain why the Backward Compatibility Check failed?
I want to fix it, but it's not clear from the output what is to be fixed.

@Ocramius
Copy link
Member

@alexmerlin check bottom of logs: you removed it from Session, which is a breakage

Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
@gsteel
Copy link
Member

gsteel commented Aug 16, 2024

Hey @alexmerlin - You can't add methods to an interface without it being considered a BC break, because you are forcing implementors to add the method to their implementation.

The patch to 1.x should only include the deprecation notice. All the changes need to go into the 2.0.x branch which doesn't exist yet.

I'll create the 2.0.x branch now so you can open a different PR to make the changes 👍

@gsteel
Copy link
Member

gsteel commented Aug 16, 2024

Actually, hang fire on the 2nd patch - let's get 1.15.0 released first…

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks @alexmerlin

@gsteel gsteel self-assigned this Aug 16, 2024
@gsteel gsteel added this to the 1.15.0 milestone Aug 16, 2024
@gsteel gsteel merged commit 5b027d0 into mezzio:1.15.x Aug 16, 2024
12 checks passed
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.

3 participants