Revise build process to reduce amount of file moves and preparation #155
Conversation
knative-prow-robot
added
approved
Co-Authored-By: Matt Moore <[email protected]>
|
Preview site is here: https://5e8672e0ad5605000693ba15--knative.netlify.com |
|
Related: #158 |
There was a problem hiding this comment.
Sorry Evan! I just saw this PR pop up in my inbox. Awesome idea to move this processing into the Hugo layouts! Unfortunately, I have zero bandwidth lately so I have NOT tried any of this but I did try to add some comments that might help.
Re: "to get a diff" - Have you tried running two local builds, one using old scripts and another with these changes, and then comparing the two output folders? (they should be the same right? also note my comment about keeping unwanted files out of the directory that Hugo builds from because they will show up in the site too, many times in unwanted ways).
scripts/processsourcefiles.sh
Outdated
| git clone --quiet -b "release-0.10" https://github.com/"$FORK"/docs.git temp/release/v0.10 | ||
| mv temp/release/v0.10/docs content/en/v0.10-docs | ||
| mkdir -p docs # -p won't fail if the file exists | ||
| for r in $(seq 13 10); do |
There was a problem hiding this comment.
Can we make this smart enough to just always look for the "latest version" - 3? That way we only ever have to update the latest version number and the site will automatically build and publish only the last three archives to knative.dev?
There was a problem hiding this comment.
That would be wonderful, but I don't know where we keep that information. I can probably make it "N" and "N-3", so there's only a single value to track. Thanks for the suggestion.
There was a problem hiding this comment.
I was slowly moving towards adding these smarts and pulled out the "latest version"/DEFAULTBRANCH into this file (also makes it easy to configure your own personal Netlify builds to your fork): https://github.com/knative/website/blob/master/scripts/docs-version-settings.sh
| @@ -30,6 +30,10 @@ then | |||
| mv content/en/docs content/en/development | |||
| # DOCS BRANCHES | |||
| echo '------ Cloning all docs releases ------' | |||
|
|
|||
| # TODO(Evan): Avoid the need for `mv` above and make the below just ap | |||
There was a problem hiding this comment.
I had to use mv because I couldnt figure out how to make "git clone" happy when trying to extract only the /docs/ folders out of each knative/docs archive branch (there are all the other files and folders in the repo that we dont want to end up in the site build).
There was a problem hiding this comment.
I clone the docs outside of /content/en, and then symlink deeper into the tree. Since the clone isn't in /content/en, it isn't published at all.
There was a problem hiding this comment.
nice. this then renders my concern below needless (content not in the publishing path wont get picked up automatically)
| @@ -1,7 +1,24 @@ | |||
| <div class="td-content"> | |||
| <h1>{{ .Title }}</h1> | |||
| {{ with .Params.description }}<div class="lead">{{ . | markdownify }}</div>{{ end }} | |||
| {{ .Content }} | |||
| {{ $out := .Content}} | |||
There was a problem hiding this comment.
Did you also test the local build too? Early on there was a lot of discrepancies between how Netlify built using Hugo and how the default local Hugo server built our content, thus the move to "building" the same way in both places.
There was a problem hiding this comment.
Yes, I've been using local build to test.
| find . -type f -path '*/content/*.md' ! -name '*_index.md' ! -name '*README.md' \ | ||
| ! -name '*serving-api.md' ! -name '*eventing-contrib-api.md' ! -name '*eventing-api.md' \ | ||
| ! -name '*build-api.md' ! -name '*.git*' ! -path '*/.github/*' ! -path '*/hack/*' \ | ||
| ! -path '*/node_modules/*' ! -path '*/test/*' ! -path '*/themes/*' ! -path '*/vendor/*' \ |
There was a problem hiding this comment.
Im very close to Go illiterate but I didnt see these files obviously ignored up in the Hugo layout?
For anything here that is explicitly called out, its because Hugo will build that file into the site and add empty/no title Nav entries into our TOC. They rendered as blank spaces and are clickable but since they lacked front matter (to either add the missing meta or explicitly exclude it from the build) they get added to the site in unwanted and conflicting ways.
There was a problem hiding this comment.
I figured out what these actually mapped to and made sure that they rendered correctly as best as I could tell.
| @@ -1,7 +1,24 @@ | |||
| <div class="td-content"> | |||
| <h1>{{ .Title }}</h1> | |||
| {{ with .Params.description }}<div class="lead">{{ . | markdownify }}</div>{{ end }} | |||
| {{ .Content }} | |||
| {{ $out := .Content}} | |||
| {{ if not ( in (slice "README.md" "_index.md") .File.LogicalName ) }} | |||
There was a problem hiding this comment.
Will this exclude hardcoded links to other non-Knative Repo's README.md files? Im not sure we still have or need to accommodate those links that point out to another Repo's README file today but maybe we should avoid changing those URLs if or when someone does add on in the future?
There was a problem hiding this comment.
This is determining if this file's name is "README.md" or "_index.md". But I've been adding "index.md" files, so I probably need to update this.
Thanks for asking!
| mv temp/release/v0.10/docs content/en/v0.10-docs | ||
| mkdir -p docs # -p won't fail if the file exists | ||
| for r in $(seq 13 10); do | ||
| CLONE="docs/release-0.${r}" |
There was a problem hiding this comment.
Should this be a variable too for that day when Knative graduates to a MAJOR version?
There was a problem hiding this comment.
When that happens, I hope to rewrite more of this anyway.
There was a problem hiding this comment.
ive changed this to use the values set in https://github.com/knative/website/blob/master/scripts/docs-version-settings.sh (so on release time, this file does not need to get updated to include the recent version number)
| @@ -152,7 +134,7 @@ find . -type f -path '*/content/*/*/*' -name 'README.md' \ | |||
| ! -path '*/contributing/*' ! -path '*/v0.6-docs/*' ! -path '*/v0.5-docs/*' \ | |||
| ! -path '*/v0.4-docs/*' ! -path '*/v0.3-docs/*' ! -path '*/.github/*' ! -path '*/hack/*' \ | |||
| ! -path '*/node_modules/*' ! -path '*/test/*' ! -path '*/themes/*' ! -path '*/vendor/*' \ | |||
| -execdir bash -c 'if [ -e _index.md ]; then echo "_index.md exists - skipping ${PWD#*/}"; else mv "$1" "${1/\README/\index}"; fi' -- {} \; | |||
| -execdir bash -c 'if [ -e index.md -o -e _index.md ]; then echo "index.md or _index.md exists - skipping ${PWD#*/}"; else mv "$1" "${1/README/index}"; fi' -- {} \; | |||
There was a problem hiding this comment.
It doesnt apply here but I thought I'd mention that Hugo does weird stuff when both an index.md and an _index.md file exist in the same directory (the build only works as expected when only one or the other exist -> an index.md file can be used and reside alone in a folder if thats the only markdown file being build from that folder).
There was a problem hiding this comment.
This is just avoiding overwriting an index.md with README.md. This actually went in #161 to support knative/docs#2377
knative-prow-robot
added
the
needs-rebase
|
FYI, im back but unfortunately only at limited capacity (currently only on Fridays). Ill start here (soon hopefully) and run this through some tests to see if we can get these upgrades in. |
knative-prow-robot
removed
the
needs-rebase
|
@evankanderson I was able to get the build for the archived versions running locally with the last commit that I added but the Netlify build never recognizes the symlinks (before and after my commit to this PR) and its unclear if the archived doc versions were working before? |
|
I think I've figured this out. Ive got both my fork and local build working by swapping out the Im still testing page links and will commit my fix here if all things look good. |
- switch `for` to `while` to make netlify happy - combine /doc/ clone step with archives+symlinks step - make sure symlinks are recreated if dir exists - use the variables in docs-version-settings.sh
knative-prow-robot
added
size/L
|
@evankanderson PTAL and clean up / improve as needed. I added comments in each of my commits. Overall i got the archive builds and symlinks to work on netlify. ISSUE: I dont think all the Im going to log off now but ill try to get back to this early next week. |
|
Digging a bit more, I think all the broken links are isolated to all of our I don't think the content of anything "included" gets processed through the updated content.html file? Im guessing the content from the readfile shortcode gets dumped in after? Is it possible to add something similar in the |
|
I think I fixed the readfile.md shortcode. Ive tested this locally and it seams to be working as expected. Ill wait for the Netlify build to test that output. |
There was a problem hiding this comment.
It looks like i've got this all working.
@evankanderson PTAL and also verify and fix and update my choices to improve etc.
If you are okay with everything, ill add the LGTM
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, RichieEscarez 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 |
|
@mattmoor would you be able to review and validate the changes here? |
knative-prow-robot
added
the
needs-rebase
|
@evankanderson: PR needs rebase. 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. |
This is part 1 of a several-step process to move the site build mostly out of shell and into golang.
This produces three benefits:
I've clicked around in multi-build mode a fair bit and this seems to work, but if there's a good way to actually diff the sites, I'd love to learn it.