search

caddyserver / cache-handler

Distributed HTTP caching module for Caddy
Apache License 2.0
262 stars 19 forks source link

feat(core): Bump to Souin v1.6.2 #23

Closed darkweak closed 2 years ago

darkweak commented 2 years ago

Closes #19 Closes #21 Closes #22 Closes #25

francislavoie commented 2 years ago

I think we should have the people who reported the linked issues to try this before closing the issues. Are we sure their problems are fixed or are you just guessing?

Re #21, my suggestion hasn't been addressed, i.e. using go-humanize to parse size values in the Caddyfile. I think that's very important to do, to aid with ease of use and for consistency with config elsewhere in the Caddyfile.

darkweak commented 2 years ago

As I said in the https://github.com/darkweak/souin/pull/190#issuecomment-1065830215 this is a low-level configuration. I prefer to be as close as possible to the Badger declaration but a PR is under preparation to use go-humanize to parse sizes directly in their Options.

francislavoie commented 2 years ago

If you're satisfied with the changes, IMO you can go ahead and merge it. Probably best to just get people to try the changes and report if they have any issues with it.

But I don't have push/merge rights on this repo, so I don't really have any authority here 😛

darkweak commented 2 years ago

@francislavoie I already asked them to try on the Souin repository before applying changes here and they confirmed that works and as cache-handler and the caddy middleware integration in Souin are equal it's good 😄