Skip to content

Conversation

@kesselb
Copy link
Contributor

@kesselb kesselb commented Jun 25, 2025

Summary

The code style update in pull request #53625 appears to trigger a Psalm warning about NullCache returning null, which makes it non-compliant with the interface. This seems like a valid finding; however, I am not familiar with this part, and a thorough review should be done.

Checklist

@kesselb kesselb self-assigned this Jun 25, 2025
@kesselb kesselb added 3. to review Waiting for reviews technical debt labels Jun 25, 2025
@kesselb kesselb requested a review from a team as a code owner June 25, 2025 12:42
@kesselb kesselb requested a review from provokateurin June 25, 2025 12:43
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

LGTM, just one of those cases where null/false were badly mixed in legacy code.

@provokateurin provokateurin enabled auto-merge June 25, 2025 13:22
@AndyScherzinger AndyScherzinger changed the title fix(nullcache): make get complaint with the interface fix(nullcache): make get compliant with the interface Jun 25, 2025
The interface defines ICacheEntry|false, thus we should not return null.

Signed-off-by: Daniel Kesselberg <[email protected]>
@kesselb kesselb force-pushed the debt/noid/wrong-return-type-nullcache branch from ea4250b to 53fb05e Compare June 26, 2025 07:55
@skjnldsv skjnldsv disabled auto-merge June 26, 2025 10:44
@skjnldsv skjnldsv merged commit beae65a into master Jun 26, 2025
192 of 194 checks passed
@skjnldsv skjnldsv deleted the debt/noid/wrong-return-type-nullcache branch June 26, 2025 10:44
@skjnldsv skjnldsv mentioned this pull request Aug 19, 2025
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.

4 participants