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: Use of undefined variables #4

Merged
merged 2 commits into from
Jan 3, 2018
Merged

fix: Use of undefined variables #4

merged 2 commits into from
Jan 3, 2018

Conversation

epage
Copy link
Contributor

@epage epage commented Dec 21, 2017

For now, cobalt is becoming stricter on the user of undefined variables to help catch problems when migrating sites through cobalt-org/cobalt.rs#346.

With this PR, your site correctly builds against 346 after running cobalt migrate. Hopefully my guesses for how to change your site are close enough :).

I will follow up with a PR that migrates your site once 346 is dev complete. Feel free to start playing with it and providing feedback (particularly around cobalt-org/cobalt.rs#323).

Its unclear what method you are using for deployment. I'd recommend ensuring that it locks down the version of cobalt used.

@badboy
Copy link
Owner

badboy commented Dec 21, 2017

Thanks!
It does remove the date display though. Was that intented?
What does the route variable does?

Deployment uses a fixed installed version of cobalt, so it’s locked down

@epage
Copy link
Contributor Author

epage commented Dec 21, 2017

What does the route variable does?

Your menu snippet uses route to specially highlight which section of the site you are in.

See https://github.com/badboy/fnordig.de/blob/master/_layouts/_menu.liquid

It does remove the date display though. Was that intented?

If I understood your site correctly, I removed it from _layouts/simple.liquid which looks like its used on pages that don't have a date. It still exists on _layouts/posts.liquid.

So before my change, I think you have pages that have <time pubdate="pubdate"></time><br />

For example, see view-source:https://fnordig.de/posts/

@badboy
Copy link
Owner

badboy commented Dec 21, 2017

Oh right, completely forgot that I did define that :D
I'm gonna take a look, apply it locally and if everything goes as planned I'll merge it. Thanks!

@epage
Copy link
Contributor Author

epage commented Jan 1, 2018

Updated for your latest blog post

@epage epage mentioned this pull request Jan 1, 2018
@badboy
Copy link
Owner

badboy commented Jan 1, 2018

Happy new year, @epage!
Completely forgot that I never merged this. Will keep it open and do the merge tomorrow.

@badboy badboy merged commit 4d7f6db into badboy:master Jan 3, 2018
@epage
Copy link
Contributor Author

epage commented Jan 9, 2018

v0.11 is now released

@epage epage deleted the fix branch January 9, 2018 14:38
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.

2 participants