-
-
Notifications
You must be signed in to change notification settings - Fork 10
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: remove empty lines from docker-compose.varnish_extras.yaml, fixes #24 #28
Conversation
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.
For reasons of evaluation precedence, IMO docker-compose.varnish_extras.yaml
works better as a recommendation. I can't remember what the issue was. But that's the format I always recommend.
This PR does not affect the functionality of We have the same empty lines in the But it matters here because you are committing this file. |
@rfay, are you referring to this recommendation ? |
Yes, didn't realize it made it into the docs :) |
Ah, this talk about |
Well, the tests are improved! Don't know if the result is improved :) |
I leave the testing to the experts 🙂. To be honest, I don't understand how this add-on works, even after reading README.md and Varnish docs. |
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.
The way to get rid of all the extra lines is to use better go templating.
For example
{{- $project_tld := "ddev.site" -}}
{{- if .DdevGlobalConfig.project_tld }}{{ $project_tld = .DdevGlobalConfig.project_tld }}{{ end }}
The mysterious {{-
and -}}
are new discoveries for me, and I usually have some trouble getting them to work the first time. But this is where all the lines come from, and this is how to clean it up.
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.
This is certainly looking great to solve the problem at hand. There's just a bit of cleanup now I think.
- In
pre_install_actions
we should remove the olddocker-compose.varnish-extra.yaml
if it exists and contains #ddev-generated - In
removal_actions
we should removedocker_compose.varnish_extras.yaml
if it exists and has #ddev-generated
Note that to see this in action, do a curl -I <site>
to see the header that shows varnish has injected itself:
rfay@rfay-tag1-m1:~/workspace/d10$ curl -I https://d10.ddev.site
HTTP/2 200
accept-ranges: bytes
age: 0
cache-control: must-revalidate, no-cache, private
content-language: en
content-type: text/html; charset=UTF-8
date: Tue, 09 Jul 2024 21:09:47 GMT
expires: Sun, 19 Nov 1978 05:00:00 GMT
server: nginx
vary: Accept-Encoding
via: 1.1 varnish (Varnish/6.0)
x-content-type-options: nosniff
x-drupal-cache: HIT
x-drupal-dynamic-cache: MISS
x-frame-options: SAMEORIGIN
x-generator: Drupal 10 (https://www.drupal.org)
x-varnish: 163852
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.
You did loads more than was casually asked in the issue! Thanks.
The Issue
How This PR Solves The Issue
Removes empty lines using go templates
{{- -}}
Renames
docker-compose.varnish-extras.yaml
todocker-compose.varnish_extras.yaml
Manual Testing Instructions
Automated Testing Overview
Related Issue Link(s)
Release/Deployment Notes