-
Notifications
You must be signed in to change notification settings - Fork 8
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 advagg and add support Drupal 10.1 #28
base: master
Are you sure you want to change the base?
Conversation
I will roll out |
Tested on Drupal 10.1.2 with latest Advagg module, seems to be fine When only Advagg aggregation is enabled, files are stored in When core aggregation is enabled -> in |
The advagg issue is https://www.drupal.org/project/advagg/issues/3308099 Original CR https://www.drupal.org/node/2888767#nginx-php-fpm |
@Sergey-Orlov are you sure that advagg storing files in |
Yes, just have re-checked (and also checked code of 5.x and 6.x versions of advagg) so we'd fix config with location |
@@ -55,23 +55,16 @@ server { | |||
open_file_cache_errors off; | |||
} | |||
|
|||
location ~* /sites/.*/files/advagg_css/ { | |||
location ~* /sites/.*/files/optimized/(css|js)/ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sergey-Orlov maybe that change is for older advagg as 5.x&6.x will work via files/(css|js|styles)/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4.x also using different dir https://git.drupalcode.org/project/advagg/-/blob/8.x-4.x/src/Asset/AssetOptimizer.php?ref_type=heads#L284
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found - it was for 7.x https://git.drupalcode.org/project/advagg/-/blob/7.x-2.x/README.md#nginx-configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, then I think we don't need separate declaration of location files for advagg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except the case if our image is still used with Drupal7 which could be true
Closes #27