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

jekyll: Do not use strict mode #14672

Conversation

EricCousineau-TRI
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI commented Feb 18, 2021

Use contains for explicit nested config check for one missing var
See: Shopify/liquid#1034 (comment)

From failure: https://drake-cdash.csail.mit.edu/test/129053560

  Liquid Exception: Liquid error ({runfiles}/drake/doc/_includes/header.html line 15): undefined variable subfolderitems included in /_layouts/default.html
jekyll 3.8.6 | Error:  Liquid error ({runfiles}/drake/doc/_includes/header.html line 15): undefined variable subfolderitems included 
/usr/lib/ruby/vendor_ruby/liquid/variable_lookup.rb:62:in `block in evaluate': Liquid error ({runfiles}/drake/doc/_includes/header.html line 15): undefined variable subfolderitems included  (Liquid::UndefinedVariable)

This change is Reviewable

Copy link
Contributor

@jamiesnape jamiesnape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers

@EricCousineau-TRI
Copy link
Contributor Author

@drake-jenkins-bot linux-focal-unprovisioned-gcc-bazel-experimental-documentation please

Copy link
Contributor Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+@jamiesnape (retroactive)
+@jwnimmer-tri for platform, please (unless you want to force-merge Jamie)
+(priority: emergency)

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform) (waiting on @EricCousineau-TRI and @jwnimmer-tri)

a discussion (no related file):
Working: Requires success from: https://drake-jenkins.csail.mit.edu/job/linux-focal-unprovisioned-gcc-bazel-experimental-documentation/4/


Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: platform.

That is all ye know on earth.

Reviewed 1 of 1 files at r1.
Reviewable status: 1 unresolved discussion (waiting on @EricCousineau-TRI)

@jamiesnape
Copy link
Contributor

I guess we let the documentation build finish since it is running.

@jamiesnape
Copy link
Contributor

CI is already noisy enough today that one extra ping is not going to be an issue if someone else merges something.

@EricCousineau-TRI
Copy link
Contributor Author

That is all ye know on earth.

I only know of Keats through the Hyperion series; maybe I'll learn more through this campaign :P

@EricCousineau-TRI
Copy link
Contributor Author

@drake-jenkins-bot linux-focal-unprovisioned-gcc-bazel-experimental-documentation please

Copy link
Contributor Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion (waiting on @EricCousineau-TRI, @jamiesnape, and @jwnimmer-tri)

a discussion (no related file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Working: Requires success from: https://drake-jenkins.csail.mit.edu/job/linux-focal-unprovisioned-gcc-bazel-experimental-documentation/4/

K, I believe this patch will fail in CI. Trying with chroot via Singularity containerization:
https://gist.github.com/EricCousineau-TRI/ee8a89e0164dab5b58766bc45947bea9

Give me 5 more min. Then I'll give up.


Use `contains` for explicit nested config check for one missing var
See: Shopify/liquid#1034 (comment)
@EricCousineau-TRI EricCousineau-TRI changed the title jekyll: Use contains for explicit nested config check jekyll: Do not use strict mode Feb 18, 2021
Copy link
Contributor Author

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion (waiting on @EricCousineau-TRI, @jamiesnape, and @jwnimmer-tri)

a discussion (no related file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

K, I believe this patch will fail in CI. Trying with chroot via Singularity containerization:
https://gist.github.com/EricCousineau-TRI/ee8a89e0164dab5b58766bc45947bea9

Give me 5 more min. Then I'll give up.

K, was able to repro error here: https://drake-cdash.csail.mit.edu/test/129305583

Fixed it in r3, it now serves the website. I'm calling this done.


Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),jamiesnape

@EricCousineau-TRI
Copy link
Contributor Author

Skipping slow builds.

@EricCousineau-TRI EricCousineau-TRI merged commit f4941ec into RobotLocomotion:master Feb 18, 2021
josephsnyder pushed a commit to EricCousineau-TRI/drake that referenced this pull request Mar 16, 2021
Use `contains` for explicit nested config check for one missing var
See: Shopify/liquid#1034 (comment)
@jamiesnape jamiesnape removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants