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

Removal of increment and decrement functionality #294

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

boesing
Copy link
Member

@boesing boesing commented Feb 28, 2024

Q A
BC Break yes

Description

As per #117, the increment and decrement functionality (at least decrement) leads to issues with several backends. This commit removes that functionality at all as it should be either part of specific adapters instead (via dedicated interface) or as part of project specific code. This cache component can not guarantee increment/decrement for all adapters out there (especially for adapters which do not support this feature natively to prevent race conditions).

@boesing boesing added this to the 4.0.0 milestone Feb 28, 2024
@boesing boesing changed the base branch from 3.13.x to 4.0.x February 28, 2024 20:21
@boesing boesing linked an issue Feb 28, 2024 that may be closed by this pull request
As per laminas#117, the increment and decrement functionality (at least decrement) leads to issues with several backends. This commit removes that functionality at all as it should be either part of specific adapters instead (via dedicated interface) or as part of project specific code. This cache component can not guarantee increment/decrement for all adapters out there (especially for adapters which do not support this feature natively to prevent race conditions).

Signed-off-by: Maximilian Bösing <[email protected]>
@boesing
Copy link
Member Author

boesing commented Feb 28, 2024

BC compatibility check is expected, this PR introduces a BC break on purpose.


@Ocramius does it make sense to add a way that the BC check is passing once BC Break label is added to a PR?
I mean, I guess its fine that the check is red here as it actually does introduce a BC break but it still feels kinda "wrong" as it is expected. So this is just an idea I'd like to bounce.

@boesing
Copy link
Member Author

boesing commented Feb 28, 2024

  • Remove event listeners listening to incrementItem, incrementItems, decrementItem and decrementItems events

@Ocramius
Copy link
Member

BC compatibility check is expected, this PR introduces a BC break on purpose.

@Ocramius does it make sense to add a way that the BC check is passing once BC Break label is added to a PR? I mean, I guess its fine that the check is red here as it actually does introduce a BC break but it still feels kinda "wrong" as it is expected. So this is just an idea I'd like to bounce.

IMO NO: can just decide to merge with red, which is a clear/visible flag

@boesing boesing merged commit 2471677 into laminas:4.0.x Mar 1, 2024
17 of 18 checks passed
@boesing boesing deleted the removal/increment-decrement branch March 1, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: Removal of increment/decrement functionality
2 participants