Fix Context.with(null) edge-case#11672
Conversation
bric3
left a comment
There was a problem hiding this comment.
Found while discussing this part of the codebase with Claude
Nice
This comment has been minimized.
This comment has been minimized.
…ps all other elements.
07acb4c to
2a7aa0c
Compare
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
There was a problem hiding this comment.
Make sense. Thanks for the fix.
Hopefully it won’t impact products / features that depended on the wrong behavior.
Note that some other with API throws exceptions with null keys (so another option would be to through exception too with null ImplicitContextKeyed objects) but I feel silently ignoring the value is still the right thing to do as this method is supposed to bring convenience...
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
Fix bug where calling
context.with((ImplicitContextKeyed) null)drops all other elements.Motivation
Found while discussing this part of the codebase with Claude
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]