- 🇳🇿New Zealand quietone
This was a bugsmash daily target. I ran the test that is in the patch and it fails, so I assumed this still needed to be done. larowlan confirmed that in slack.
This also needs an issue summary update, see Write an issue summary for an existing issue → for guidance.
- Status changed to Needs review
almost 2 years ago 4:31am 1 March 2023 - 🇮🇳India bhaveshithape
Adding Rerole for d10, As previous patch failed to apply on d10 and also most of the files already had the changes so there is minor change in the patch.
- Status changed to Needs work
almost 2 years ago 4:47pm 3 March 2023 - 🇺🇸United States smustgrave
Regarding #46
also most of the files already had the changes
How so? I'm checking D10.1 and I don't see the changes for a few of those files. Also this lost test coverage so adding that tag back
Was previously tagged for issue summary which still needs to happen as well.
- First commit to issue fork.
- Merge request !6054Resolve #2509436 "Make cachepluginbasegenerateresultskey to" → (Open) created by dimitriskr
- Status changed to Needs review
12 months ago 4:32pm 7 January 2024 - 🇬🇷Greece dimitriskr
I've altered the existing test to support the new format brought by
$cache_context->getContext()
- Status changed to Needs work
12 months ago 3:49pm 9 January 2024 - 🇺🇸United States smustgrave
Issue summary section for solution is missing.
Tagging for submaintainer to make sure they agree on the approach
- Status changed to Needs review
12 months ago 7:51pm 10 January 2024 - Status changed to Needs work
12 months ago 8:59pm 10 January 2024 - 🇨🇭Switzerland berdir Switzerland
Those asserts are not doing anything, see MR comment.
- Status changed to Needs review
11 months ago 7:44pm 18 January 2024 - Status changed to Needs work
11 months ago 3:04pm 30 January 2024 - 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
I feel like #27.1 was never addressed. My assumption is also that if CacheableMetadata::merge() has
$result = clone $this;
, then it seems kind of moot toassert($result instanceof CacheableMetadata);
.Comment #28 also touched on the ampersand but not sure if we really need that.
From a cache submaintainer point of view, I see nothing wrong here. Assertions are harmless, casting casche context return values to strings is too because worst case scenario that leads to a few cache entries no longer being reachable, at which point they get recalculated and stored again. Only been subsystem maintainer for a few months now, so will ask @catch before removing tag.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
This looks fine conceptually, still NW per my latest comment, though.