Updates Hugo & removes vendored docsy #284
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,14 @@ | |
| position: fixed; | ||
| top: 0; | ||
| min-height: 71px; | ||
| z-index: var(--of--header-main); | ||
| display: grid; | ||
| grid-template-columns: max-content 1fr max-content; | ||
| align-items: center; | ||
| grid-template-areas: | ||
| "brand search search" | ||
| "nav nav nav"; | ||
|
|
||
|
There was a problem hiding this comment. There are a LOT of these sprinkled all over, which makes it really hard to focus on the actual changes. Lot of brain power is just needed for filtering out the noise. I'm guessing this is a text editor setting that needs to be fixed There was a problem hiding this comment. @anik120 I agree - people who commit a bunch of spaces at the end of lines should fix their editors :) In the meantime - Github UI also has a setting for this. I hope this helps. 
|
||
| @media (min-width: $ov--breakpoint--lg) { | ||
| display: flex; | ||
| align-items: center; | ||
|
|
@@ -21,7 +21,7 @@ | |
| grid-area: brand; | ||
| display: flex; | ||
| margin: var(--of--spacer--sm); | ||
|
|
||
| @media (min-width: $ov--breakpoint--lg) { | ||
| &__picture { | ||
| min-width: 177px; | ||
|
|
@@ -67,10 +67,10 @@ | |
| grid-template-columns: 1fr 1fr; | ||
| grid-template-rows: min-content min-content; | ||
| @media (min-width: $ov--breakpoint--lg) { | ||
| display: flex; | ||
| margin: 0; | ||
| } | ||
|
|
||
| &__li { | ||
| @media (min-width: $ov--breakpoint--lg) { | ||
| border: none; | ||
|
|
@@ -85,10 +85,10 @@ | |
| &:not(:last-of-type):not(:nth-last-child(-n+2)){ | ||
| border-bottom: none; | ||
| } | ||
|
|
||
| } | ||
| &__a { | ||
| width: 100%; | ||
| display: inline-block; | ||
| position: relative; | ||
| padding: var(--of--spacer--xs) var(--of--spacer--md); | ||
|
|
@@ -106,7 +106,7 @@ | |
| // &.has-dropdown { | ||
| // grid-area: link; | ||
| // } | ||
|
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -192,14 +192,14 @@ flowchart TB | |
|
|
||
| A(v0.0.1) --> B(v0.0.4) | ||
| {{</mermaid>}} | | ||
| | skips | `ID(<bundle tag>) x--x \| <versions that should be skipped> \| ID(<bundle tag>)` | {{<mermaid>}} | ||
|
There was a problem hiding this comment. Newer versions of Hugo use goldmark to render markdown with an extension which implements GitHub Flavored Markdown: Tables spec. Accordingly to the spec pipe in a cell’s content needs be escaping (example). As a result of proper spec implementation in goldmark tables & graphs get broken with new versions of Hugo. |
||
| flowchart TB | ||
| classDef head fill:#ffbfcf; | ||
| classDef installed fill:#34ebba; | ||
|
|
||
| A(v0.0.1) x--x |v0.0.2,v0.0.3| B(v0.0.4) | ||
| {{</mermaid>}} | | ||
| | skipRange | `ID<bundle tag>) o--o \| <range> \| ID(<bundle tag>)` | {{<mermaid>}} | ||
| flowchart TB | ||
| classDef head fill:#ffbfcf; | ||
| classDef installed fill:#34ebba; | ||
|
|
@@ -213,14 +213,14 @@ flowchart TB | |
|
|
||
| A(v0.0.1) -.-> B(v0.0.4) | ||
| {{</mermaid>}} | | ||
| | future skips | `ID(<bundle tag>) x-.-x \| <versions that should be skipped> \| ID(<bundle tag>)` | {{<mermaid>}} | ||
| flowchart TB | ||
| classDef head fill:#ffbfcf; | ||
| classDef installed fill:#34ebba; | ||
|
|
||
| A(v0.0.1) x-.-x |v0.0.2,v0.0.3| B(v0.0.4) | ||
| {{</mermaid>}} | | ||
| | future skipRange | `ID<bundle tag>) o-.-o \| <range> \| ID(<bundle tag>)` | {{<mermaid>}} | ||
| flowchart TB | ||
| classDef head fill:#ffbfcf; | ||
| classDef installed fill:#34ebba; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| module github.com/operator-framework/olm-docs | ||
|
|
||
| go 1.19 | ||
|
|
||
| require github.com/google/docsy v0.6.0 // indirect |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| github.com/FortAwesome/Font-Awesome v0.0.0-20220831210243-d3a7818c253f/go.mod h1:IUgezN/MFpCDIlFezw3L8j83oeiIuYoj28Miwr/KUYo= | ||
| github.com/google/docsy v0.6.0 h1:43bVF18t2JihAamelQjjGzx1vO2ljCilVrBgetCA8oI= | ||
| github.com/google/docsy v0.6.0/go.mod h1:VKKLqD8PQ7AglJc98yBorATfW7GrNVsn0kGXVYF6G+M= | ||
| github.com/google/docsy/dependencies v0.6.0/go.mod h1:EDGc2znMbGUw0RW5kWwy2oGgLt0iVXBmoq4UOqstuNE= | ||
| github.com/twbs/bootstrap v4.6.2+incompatible/go.mod h1:fZTSrkpSf0/HkL0IIJzvVspTt1r9zuf7XlZau8kpcY0= |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,18 +3,8 @@ set -ev | |
|
|
||
| CONTAINER_RUN_EXTRA_OPTIONS=${CONTAINER_RUN_EXTRA_OPTIONS:=""} | ||
| CONTAINER_ENGINE=${CONTAINER_ENGINE:="docker"} | ||
|
|
||
|
Comment on lines
-15
to
-16
There was a problem hiding this comment. See comment in |
||
| # ignore | ||
| # 1: links going back to help.github.com are rate-limited and can make this flaky | ||
| # 2: docsy autogenerated edit links to original markdown source, which will fail if the markdown file is new | ||
| ${CONTAINER_ENGINE} run --rm -v $(pwd)/public:/target mtlynch/htmlproofer /target --ignore-empty-alt --ignore-status-codes 429 --allow-hash-href --ignore-urls '/help.github.com/,/edit\/master\/.*\.md/,/tree\/master\/.*\.md/,/new\/master\/.*filename=change-me\.md/' | ||
|
There was a problem hiding this comment. This implies that we now have to have a local install of hugo to run, right? (e.g. I guess this goes back to my earlier comment about how close to hugo HEAD we need to be, because it's just 0.101.0 < 0.113.0 that isn't exposed in the available containers. There was a problem hiding this comment. @grokspawn that was actually my initial goal (not of this PR, but overall): I wanted to be able to run this locally.
Even ignoring There was a problem hiding this comment. Created #301 to track these other updates needed, so that this PR doesn't get any more complex. 😄 There was a problem hiding this comment. Okay. Sounds like we can avoid a massive PR like this if we just update the README then?
This is the submodule: https://github.com/operator-framework/olm-docs/tree/master/themes/docsy, which you do remove in a commit further up the chain too. Removing the theme from being committed in the repo: what benefit do we get other than saving a little space in the git sever? I.e looking for justification for investing building/review cycles for this effort. The mention of https://github.com/google/docsy-example.git is clearly a typo, if you read the statement above, it's trying to show the example of cloning the olm-docs repo, but with
It says you must have version 0.45 at least. I am going to try installing Hugo on my machine from that link above and go through the steps to see how things go. There was a problem hiding this comment.
No. This whole thing started because I couldn't run the docs site locally. I was forced to create a draft PR and wait for CI to build and publish a preview for me. After spending some time trying to make old version of Hugo work on
No, this is not a submodule. Maybe it used to be a submodule, but at some point someone committed it and made changes to the vendored code.
I have this in the PR description:
I'm not concerned about saving space on GitHub (especially given that this removal doesn't remove anything from the history and technically doesn't save any space). What I'm concerned about is when people accidentally modifying vendored code. It is like committing vendored go modules into your repo and then making changes in
Justification is - I couldn't run the thing locally. Not a great dev experience. On top of that - we are using a legacy service (docsearch v2). This is not to mention a bunch of other related tech debt reasons. After this change we can fix some pain points and open the door for further improvements. For example we can share a theme between sdk.operatorframework.io and olm.operatorframework.io: the only difference is a few colours. This way we can maintain a consistent look in feel between the two.
There is no binary for Instead of spending time on this I suggest that we proceed with the changs and leave 5 years old version of Hugo behind. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we were running Hugo in a container (see
hack/ci/link-check.sh) usingklakegg/hugoimage, but new versions of Hugo are missing. So now we are installing hugo from github releases.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no particular problem with the idea of shifting the mechanism if we need to, but I see versions up to 0.101.0 available in docker, so the intention was to access versions closer to HEAD (0.113.0) here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grokspawn this PR updates Hugo to 0.111.1. It was the latest version at the moment of creating of this PR. I'm happy to update it to the latest version once we merge this PR. It should be significantly smaller PR.
As far as I see Hugo doesn't maintain official docker image and
klakegg/hugois maintained by an individual. Despite most likely there is nothing wrong with it - i'm hesitant to use unofficial image.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is pretty popular on GitHub though: https://github.com/klakegg/docker-hugo
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anik120 I mean... This package is also popular - https://www.npmjs.com/package/is-odd. It got 431k downloads each last week. What it basically does is
num % 2 == 1. What are you implying?When there is a choice between a poorly maintained random docker image from the internet vs 2 lines of bash which get an official release artefact... Choice is obvious to me.