Fail the build on undefined variables #248
Conversation
_layouts/base.html
Outdated
| @@ -28,13 +28,13 @@ | |||
| <link rel="stylesheet" type="text/css" media="screen" href="/css/app/code_of_conduct.css"> | |||
| <link rel="stylesheet" type="text/css" media="screen" href="/css/app/ribbon.css"> | |||
|
|
|||
| <title>{% if page.title %}{{ page.title }} | {% endif %}Seattle GNU/Linux Conference</title> | |||
| <title>{{ page.title }}{% unless page.title contains site.name %} | {{ site.name }}{% endunless %}</title> | |||
There was a problem hiding this comment.
This now requires each page to have title, partly because I didn’t see a way to check for their existence but also because we should be writing titles anyway.
|
These changes makes sense to me, but I don't feel confident in doing a code-review. Trust your judgement if they are ready to go live. |
|
I think it’s mostly a matter of preference;
|
|
Gotcha, I am pretty fine with things being held back for corrections, as long as notification is raised. In terms of un-parameterized content, can you give an example? Perhaps I didn't read the past objections as closely as I should have... |
|
#247 initially included fa9c023 to avoid rewriting history with mangled links, but Ben suggested omitting this step, you +1’d his position, Nathan defended it, and I yielded to consensus. I still don’t really understand the objection so it’s hard for me to predict whether it also applies to this PR. |
|
Still going in circles trying to figure this out, I blame some of the double negatives in comments related to that issue... IMHO, we want parameterization in general, we just don't want to have Freenode linked anywhere. Removing parameters from old posts is A-okay with me. There is some world where the word wasn't parameterized, but just the link, and I think that's where some confusion perhaps came from? |
AndrewKvalheim
force-pushed
the
liquid-strict
branch
from
aa0ba41
to
f3d3707
Compare
AndrewKvalheim
force-pushed
the
liquid-strict
branch
from
f3d3707
to
d82d872
Compare
|
@AndrewKvalheim What's the status of this PR? Should it be closed at this point? |
|
Available, should we want it. Probably nice to have, but potentially a nuisance for casual editing. Personally I lean toward keeping slow issues open, but I understand that it’s also popular to hide and deliberately forget about them. |
AndrewKvalheim
force-pushed
the
liquid-strict
branch
from
d82d872
to
036ca42
Compare
Currently if you mistype a variable name or delete a variable that’s still in use Jekyll will silently ignore the error. Want to enable its strict mode to fail the build instead?
Due to Shopify/liquid#1034 working with deliberately undefined variables is a pain but I’d prefer to make the tradeoff.