chore(merch): end the A/B experiment #7672
Conversation
| @@ -109,6 +110,7 @@ export function initializeMiddleware(app) { | |||
| app.use(sameOriginMiddleware) | |||
| app.use(escapedFragmentMiddleware) | |||
| app.use(unsupportedBrowserMiddleware) | |||
| app.use(pageCacheMiddleware) | |||
There was a problem hiding this comment.
I didn't see a distinct commit that I could un/revert to target just the pageCache stuff.
So I guess the thing to do here is follow up with another PR that removes this middleware and associated code @mzikherman ?
There was a problem hiding this comment.
:ceiling-cat: This pageCacheMiddleware should be its own PR, discussed independently of product work. its a big change.
There was a problem hiding this comment.
Yea, I snuck removing the page cache as part of the AB test PR, b/c some page cache specs introspect currently running AB tests and were failing. Additionally, the page cache middleware is not enabled in production, and only was for a brief period of time a while ago. We'd discussed since that it shouldn't be used and there are better ways to accomplish it, if needed. Either way it wasn't in use.
I totally agree that removing it tho, is probably cleaner to just have done in a separate PR (it was the test conflict that caused me to do them simultaneously). My bad.
its a big change.
@damassi it shouldn't be a big change! We had already discussed previously that we didn't want it, and in fact its not enabled and hasn't been for a good while.
There was a problem hiding this comment.
Oh sorry! there's been a few revert PRs floating around, and was confused by Roops comment above about this too being a revert PR. Was thinking the middleware being removed here (via revert) was something new being added (independent of the past old work), that was now being removed and was suggesting that if we add it back it should be in its own PR for all of the old reasons discussed. All good!
|
Alright gonna merge this to end the experiment. I agree that the pageCache stuff could be addressed separately, and I'll leave that to folks more knowledgeable (which I think is the two people already in here 😄) |
anandaroop
changed the title
This deploy contains the following PRs:
|
Reverts #7638
Queueing this up now so we can just merge it when we're ready — should be any moment now.
I think this is all there is to it, right @mzikherman? Though — if we do this via revert, we'll end up recommitting some code that we wanted to remove, correct?