-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Removed obsoleted code from Umbraco.Cms.Core.Cache & .Routing #19959
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
Conversation
… its tests to use the new logic
AndyButland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @NillasKA. I just had a couple of questions that I've raised inline. My guess is you've had discussions about them internally so may all be fine, but even if so can you respond on the PR so we have answers documented please.
I note that Playwright tests are failing so that needs investigating. First thing would just be to ensure that the site runs locally correctly after these updates. If so, it may just be something transient failed and the tests can just be re-run (which will happen anyway if you push any updates).
src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs
Outdated
Show resolved
Hide resolved
tests/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformerTests.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs
Outdated
Show resolved
Hide resolved
|
I notice the failing smoke tests. I'll investigate |
AndyButland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now: can see the tests are passing and the queries I had have been resolved.
I'd just ask though if you could please get a review from @nikolajlauridsen or @Zeegaan too before merging - just to make sure they are happy with the amends made related to caching. They are more familiar than me with that and I know you've been speaking to one or both of them already in working on this.
Just to add too - there's some related clean-up work on the unit tests for content finders. I've added a new task for that (55978) and suggest it is tackled as a separate PR.
# Conflicts: # src/Umbraco.Cms.Api.Delivery/Services/ApiMediaQueryService.cs # src/Umbraco.Core/Routing/ContentFinderByUrlAlias.cs # src/Umbraco.Infrastructure/PublishedContentQuery.cs # src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs # src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs # src/Umbraco.PublishedCache.HybridCache/MediaCache.cs # tests/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformerTests.cs
Description
A ton of obsoleted code, that has been scheduled for removal in Umbraco 17 has been removed, and their usages has been adjusted (If there was any)
Mainly this PR is the removal of obsoleted code from the Umbraco.Cms.Core.Cache namespace but also quite a lot from the Umbraco.Cms.Core.Routing namespace, since a lot of it was coupled into the caching.