Conversation
Breaking changes fixed:
- markdownify on template.HTML: cast with `| string |` in admonition.html
- disableKinds: rename taxonomyTerm to term in config.toml
- mediaTypes: replace delimiter with suffixes array
- toCSS -> css.Sass, .Site.IsServer -> hugo.IsServer in css.html
- Remove unused version.html shortcode
Bulma 1.0.4 drops LibSass support, so this also:
- Installs Dart Sass via scripts/setup-dart-sass.sh (Hugo needs the
real executable, not the npm JS wrapper), invoked from `make serve`
and exported onto PATH from the Makefile for production builds
- Rewrites assets/sass/*.sass into a single style.scss, loading
Bulma's precompiled CSS via CDN (its own Sass build no longer
needs LibSass/Dart Sass) and layering only site-specific
overrides/CSS custom properties on top
- Removes yarn.lock in favor of npm (package-lock.json only)
- Adds a Dockerfile pinned to Hugo 0.163.3 + Node 20 for local/CI
parity with Netlify
Note: layouts/partials/{navbar,social-buttons}.html,
layouts/_default/baseof.html, layouts/partials/docs/{menu,sidebar}.html
are intentionally untouched here - upstream/main already carries its
own .File nil-guards and its own dark-mode/theme-toggle and CLI-docs
nav features (added independently since this work was branched), so
re-copying the stale versions from the original PR branch would have
reverted already-shipped upstream work.
Verified: `make production-build` succeeds cleanly against current
upstream/main (2261 pages, 0 errors, only pre-existing deprecation
warnings unrelated to this change).
Signed-off-by: Ross Golder <ross@golder.org>
The single-file style.scss rewrite dropped these two partials entirely, even though the JS/HTML wiring for both features (navbar's theme-toggle partial, javascript.html's table-popout.js) was left untouched and still upstream-correct. Result: both features would still activate at runtime but render completely unstyled, since their source .sass files were no longer imported by anything once style.sass was replaced. Converts the indented Sass syntax to SCSS and folds the content directly into style.scss rather than keeping them as separate imports. table-popout.sass's `outline: 2px solid $primary` (a Bulma Sass variable, unavailable now that Bulma is loaded via CDN rather than compiled from source) is replaced with the equivalent hsl(var(--bulma-primary-h), var(--bulma-primary-s), var(--bulma-primary-l)) already used elsewhere in this file for the same color. Verified by compiling the resulting style.scss directly with Dart Sass (bypassing Hugo/PostCSS, which are hitting an unrelated environment-level Node permission issue in this sandbox): 712 lines of output, dark-mode and table-popout selectors all present, the outline substitution resolves correctly. Signed-off-by: Ross Golder <ross@golder.org>
Same class of breaking change as the docs/ partials: these two templates (added by goharbor#645, after the Hugo upgrade work was originally branched) never got the .File nil-guard that Hugo 0.163 requires. Broke the /cli-docs section outright once content/cli-docs stopped being tracked in git and started coming purely from cli-docs.sh's generated output at build time - the section index page has no .File, and Hugo 0.163 errors instead of Hugo 0.74's more lenient handling. Verified: full `make production-build` succeeds (2150 pages, 0 errors), /cli-docs renders. Signed-off-by: Ross Golder <ross@golder.org>
- e2e/breaking-changes.spec.ts covers the four breaking-change fixes from the Hugo/Bulma upgrade: admonition markdownify, disableKinds taxonomy/term pages, the _redirects custom output format, and the css.Sass CSS pipeline (fingerprinted output, computed styles applied) - .github/workflows/test.yml: build -> test -> lint pipeline, Hugo 0.163.3 pinned via peaceiris/actions-hugo, Playwright browsers installed in CI - make test / make test-ui targets, README section documenting them - .gitignore: test-results/, playwright-report/ Built on top of the Hugo/Bulma upgrade branch since these tests assert against its output (fingerprinted CSS, css.Sass pipeline, disableKinds behavior) and would fail against current upstream/main. Note: local `make production-build` in this sandbox intermittently fails with a Node permission-model error inside autoprefixer's browserslist parent-directory walk, unrelated to any code here - it reproduced identically on the unmodified Hugo/Bulma upgrade branch that had built successfully minutes earlier in the same session, with no dependency or code change in between. Treating it as an environment artifact rather than a real defect; worth a clean re-run in CI to confirm. Signed-off-by: Ross Golder <ross@golder.org>
Vad1mo
left a comment
Vad1mo
left a comment
There was a problem hiding this comment.
CI for this repo is worth having, especially given there is currently no build check on PRs. A few things need fixing before this can gate merges.
Blocking:
-
The
lintjob fails immediately.grep -r '[[:space:]]$' --include='*.md' ...matches 83 already-tracked files on current main (content/blog/harbor-2.1.md,harbor-2.10.md,contributing.md, and 80 more), so the step exits 1 on every PR including this one. Either clean the tree in a separate commit or scope the check to files changed in the PR. For a docs site whose content is synced from goharbor/harbor, I would lean toward dropping this gate entirely. -
Three of the five tests read
process.cwd()/public, butplaywright.config.tsstartshugo server, which never writespublic/. In CI that directory is an artifact from a different job; locally it is whatever stale output is lying around. Those assertions are not testing the server under test. Either run the tests against ahugobuild served statically, or make them assert over HTTP. -
Related: the fingerprint test cannot pass against
hugo server. In server modehugo.IsServeris true, socss.htmltakes the dev branch and emits non-fingerprinted CSS with no integrity attribute. -
Two assertions cannot fail.
expect(getComputedStyle(body).backgroundColor).not.toBe('')is always true, since a browser returns at leastrgba(0, 0, 0, 0). And thedisableKindstest checks thatpublic/tagsandpublic/categoriesare absent, but this site configures no[taxonomies], so they are absent regardless of thetaxonomyTerm->termfix. Neither validates the breaking change it is named after.
Non-blocking:
- The admonition test hardcodes
/docs/2.1.0/administration/configuring-replication/, which is content fetched at build time from another repo. That will rot. make prepareruns twice in thebuildjob (once explicitly, once viaproduction-build's dependency), and each run clones harbor-cli and walks release branches.actions/checkout@v4is also used withoutsubmodules, while the repo has aharborsubmodule.webServer.commandinvokesscripts/setup-dart-sass.sh, which downloads a binary Hugo never actually uses (see my comment on #745).- Please pin
peaceiris/actions-hugoandactions/checkoutto commit SHAs and add an explicitpermissions:block.
Worth stating who owns these tests and whether they are intended to block merges, since that determines how much flakiness the project is signing up for.
5 participants
Summary
Split out from #731 per review feedback. Depends on #745 (Hugo/Bulma upgrade) - this branch is built on top of it, so the diff below includes #745's commits until that one merges; only the last commit here is unique to this PR.
e2e/breaking-changes.spec.tscovers the breaking-change fixes from the Hugo/Bulma upgrade: admonitionmarkdownify,disableKindstaxonomy/term pages, the_redirectscustom output format, and thecss.SassCSS pipeline (fingerprinted output, computed styles applied).github/workflows/test.yml: build -> test -> lint pipeline, Hugo 0.163.3 pinned viapeaceiris/actions-hugo, Playwright browsers installed in CImake test/make test-uitargets, README section documenting them.gitignore:test-results/,playwright-report/These tests assert against the upgraded Hugo/CSS output and would fail against current
main, which is why this can't land independently of #745.Related
Part of the work originally proposed in #731, split per @OrlinVasilev's review request. Depends on #745 (Hugo/Bulma upgrade). See also #744 (misc cleanup, independent).
Test plan