- 
                Notifications
    You must be signed in to change notification settings 
- Fork 8k
jekyll: set liquid error mode to strict #15622
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
Conversation
| ✅ Deploy Preview for docsdocker ready!Built without sensitive environment variables 
 To edit notification comments on pull requests, go to your Netlify site settings. | 
|  | ||
| # https://jekyllrb.com/docs/configuration/liquid/ | ||
| liquid: | ||
| error_mode: strict | 
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 only concern I have is that these errors may happen in external sources (eg reference YAMLdocs or Markdown we pull in), and while these should be addressed, we may not be able to do that immediately 🤔
I'm surprised that the format string in compose didn't show up in the rendered output though; ISTR that liquid would just ignore the "invalid" liquid templates, and leave them as-is (so the warnings could be ignored if it wasn't really a liquid template)
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 only concern I have is that these errors may happen in external sources (eg reference YAMLdocs or Markdown we pull in), and while these should be addressed, we may not be able to do that immediately 🤔
We have the same issue with htmlproofer I guess, not sure what we can do about it except if upstream run the same validation steps like we do in buildx repo: docker/buildx#1218
0b843a9    to
    aad35fe      
    Compare
  
    Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
aad35fe    to
    a9981de      
    Compare
  
    | ✅ Deploy Preview for docsdocker ready!
 To edit notification comments on pull requests, go to your Netlify site settings. | 
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.
SGTM
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.
👍
While working on another PR I found out there are liquid warnings introduced by #15494 in the logs: https://github.com/docker/docker.github.io/runs/8288660605?check_suite_focus=true#step:4:491
Looking at this page https://docs.docker.com/compose/install/uninstall/#inspect-the-location-of-the-compose-cli-plugin, format is indeed broken:
To avoid this kind of issues in the future, sets Liquid error mode to strict.
Also got the following issue while enabling new error mode for Liquid:
This is fixed with last commit by using
captureinstead ofassign.