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 integrity config example in README #664

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

angelikatyborska
Copy link

@angelikatyborska angelikatyborska commented Mar 5, 2022

  • I have added or updated the specs/tests.
  • I have verified that the specs/tests pass on my computer.
  • I have not attempted to bump, or alter versions.
  • This is a documentation change.
  • This is a source change.

Description

I was looking for a way to prevent <img> elements from having an integrity attribute because that's not valid HTML. I saw this config snippet in the README:

assets:
  defaults:
    integrity:
      {css,img,js}: false

I tried it out, but I got an exception (undefined method 'key?' for "highlighter":String). After reading the source code I found this hint:

Jekyll::Assets::Hook.register :config, :before_merge do |c|
c.deep_merge!({
defaults: {
img: {
height: false,
integrity: Jekyll.production?,
width: false,
},
},
})

It drove me to realize that this config will do what I want:

assets:
  defaults:
    img:
      integrity: false

Additionally, the README says that "All values listed below are default", which made me unsure if the values should be set to true or false because the real default value is Jekyll.production?. I chose true because I consider it's more important to know what happens in production, but let me know if you disagree. Maybe a comment would be necessary?

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.

None yet

1 participant