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

Fix name of opcache reset file for deployer #378

Closed
wants to merge 1 commit into from

Conversation

absumo
Copy link

@absumo absumo commented Mar 26, 2024

Allow the correct file in .htaccess.
The name changed going from capistrano to deployer.

See: https://github.com/tijsverkoyen/deployer-sumo/blob/b07c4853a06909fcea4d6bc9ba25b740c0752689/recipe/opcache.php#L9

Type

  • Bugfix

The name changed going from capistrano to deployer
@absumo absumo requested a review from a team March 26, 2024 09:17
@@ -42,7 +42,7 @@ deny from env=stayout
RewriteRule src/Backend/Core/Js/ckfinder/core/connector/php/connector\.php - [L]

# allow the php-opcache-reset.php file
RewriteRule ^php-opcache-reset\.php - [L]
RewriteRule ^php-opcache_reset\.php - [L]
Copy link
Member

Choose a reason for hiding this comment

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

Why? Wouldn't it make a lot more sense to fix the filename? With this change it is a mix of - en _

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking about backwards compatibility, changing the name in deployer would break sites where the .htaccess already was correct. But that is probably nowhere the case, so fixing it in deployer makes indeed more sense.

Copy link
Author

Choose a reason for hiding this comment

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

Closed in favor of tijsverkoyen/deployer-sumo#33

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.

2 participants