Temporarily remove Hugo Pipes Sass conversion #10373
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
@lucperkins: GitHub didn't allow me to assign the following users: kbhawkey. Note that only kubernetes members and repo collaborators can be assigned. |
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit 830343f https://deploy-preview-10373--kubernetes-io-master-staging.netlify.com |
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit e538468 https://deploy-preview-10373--kubernetes-io-master-staging.netlify.com |
| @@ -24,6 +24,9 @@ production-build: build check-headers-file ## Build the production site and ensu | |||
| non-production-build: ## Build the non-production site, which adds noindex headers to prevent indexing | |||
| hugo --enableGitInfo | |||
|
|
|||
| sass-build: | |||
| scripts/sass.sh build | |||
There was a problem hiding this comment.
minor nit: HUGO_VERSION is set in Makefile and netlify.toml. Is there a better place to set this variable that all scripts/configuration can use?
There was a problem hiding this comment.
Unfortunately there really isn't a way around that AFAIK. Netlify needs to have that information in the config and the Makefile needs it for people to be able to easily build the Docker image. Ideally the Dockerfile could extract that information from the Netlify config but I'm not sure there's a way to do that that doesn't add a lot of complexity.
|
|
||
| .open-nav, .y-enough | ||
| #tryKubernetes | ||
| width: 150px |
There was a problem hiding this comment.
Would you accept an update here or should I continue with PR#10353?
There was a problem hiding this comment.
I would say hold off on that. Wait for this PR to be merged, then move all of the Sass files in assets/sass into this root-level folder in your PR. That strikes me as the least messy option.
sass/_base.sass
Outdated
| @@ -996,8 +996,10 @@ dd | |||
| img | |||
| max-width: 100% | |||
|
|
|||
| #TableOfContents > ul > li { list-style: none; } | |||
There was a problem hiding this comment.
Did these files get moved from assets?
There was a problem hiding this comment.
@lucperkins Thanks for the workaround until netlify catches up. ✨
One request: add an OWNERS to sass/ to let @alexcontini continue working as needed (see comment on resources/OWNERS).
I'm willing to approve if other reviewers /lgtm. 👍
| @@ -490,7 +490,7 @@ Slack channel or the | |||
| If you aren't ready to create a pull request but you want to see what your | |||
| changes look like, you can use the `hugo` command to stage the changes locally. | |||
| 1. Install Hugo version `0.40.3` or later. | |||
| 1. Install Hugo version {{< hugoVersion >}} or later. | |||
resources/OWNERS
Outdated
| @@ -1,8 +0,0 @@ | |||
| # Allow CSS/SASS updates from blog and case studies owners | |||
There was a problem hiding this comment.
I do think we need to add this file verbatim to sass/ until we reenable the Sass conversion pipe, so that @alexcontini can continue making updates.
Otherwise, it makes sense to delete this, given that it only gives permissions to generated results. Sorry for not cleaning this up in #10198!
There was a problem hiding this comment.
Good call on that! Completely missed it. Done.
|
@kbhawkey 👋 Does this PR look good enough to you to |
|
/lgtm |
|
@kbhawkey: changing LGTM is restricted to assignees, and only kubernetes/website repo collaborators may be assigned issues. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zacharysarah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Background: Netlify does not yet support the "extended" version of Hugo, which includes support for Sass conversion to CSS (which is under the Hugo Pipes feature). The current workaround for this issue is to commit the generated
resourcesfolder to Git. As with most instances of committing generated assets to Git, this has produced a variety of headaches. This PR removes the Hugo Pipes Sass conversion pipeline in favor of one that uses thesassCLI tool for those who need to work with Sass. Once Netlify supports the extended version of Hugo we can go back to using Hugo Pipes.