Skip to content
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

Update readme in preparation for next caddy release #297

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

elee1766
Copy link
Contributor

@elee1766 elee1766 commented Feb 25, 2024

In future versions of caddy, filesystems have been changed to be declared in the global block, then referenced using the fs directive, see caddyserver/caddy#5833

this is a breaking change, see #288

it shouldn't break the actual plugin (if it does, let me know, and i can investigate), but the readme should be updated.

also probably shouldn't merge until the next official caddy release.

Copy link
Owner

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elee1766 thanks for the PR!

Since this change isn't released yet, would you mind adding back the original example with a note that prior to 2.8.x the old configuration has to be used?

We can remove it once 2.8+ becomes the standard, but until then, I think it would be confusing for people to have an unreleased version config in the docs.

@elee1766
Copy link
Contributor Author

yeah that makes sense. i was just going to leave this pr until the new caddy release but this makes more sense

@tetra12
Copy link

tetra12 commented Feb 26, 2024

We can remove it once 2.8+ becomes the standard, but until then, I think it would be confusing for people to have an unreleased version config in the docs.

@sagikazarmark You should create a new branch like caddy v2.8.0, update docs properly and add a note that it will be merged to main when caddy v2.8 arrives

Now my issue is fixed thanks to @elee1766 and I can build your module against caddy master

@sagikazarmark
Copy link
Owner

This just documentation, so I’m satisfied with the current version.

@sagikazarmark sagikazarmark merged commit ac6b8ed into sagikazarmark:main Feb 26, 2024
5 checks passed
@srenatus
Copy link

Five months later, I ran into this problem 😅 Thanks for helping me out indirectly here! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants