changed the NewCompressor logic to be support wildcards in the defaul…#921
changed the NewCompressor logic to be support wildcards in the defaul…#921eteran wants to merge 3 commits into
Conversation
…t set like it already did for a custom list by reusing that logic made defaultCompressibleContentTypes contain "text/*" as all text mime types are arguably very compressible added application/xml to defaultCompressibleContentTypes since it is also a common XML mime type
|
@Neurostep can you help review this please? |
|
Looks like it fails here: So perhaps I just need to update the tests now that some other things are being compressed. I'll see if I can fix it real quick |
7 regular rules and 1 wildcard in the default ruleset
|
I updated the tests and now things pass locally. However, it looks like perhaps the CI workflow didn't rerun on github? |
Neurostep
left a comment
Neurostep
left a comment
There was a problem hiding this comment.
Overall changes look good to me 👍 I wonder if we should bump the minor version here since we are going to change the default behavior of the middleware and all of a sudden for some clients the middleware is going to compress all text/* content types 🤔
|
Just wanted to follow up to see if there was anything you need on my end to help move things to the next steps. |
|
I don't mean to bug, but it's been over 9 months. @Neurostep already reviewed it once, is there any likelihood of this being re-reviewed and merged anytime soon? |
|
Closing in favor of the cleaner, more minimal PR: #1126 Thank you! |
|
Seems like an Inferior solution, but I guess better than nothing 2 years later 🤷♂️ |
|
Merged via #1127. Thank you 🎉 |
3 participants
Uh oh!
There was an error while loading. Please reload this page.