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

Reintroduce Docsy as a submodule #160

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

chalin
Copy link
Collaborator

@chalin chalin commented Aug 7, 2023

This PR reintroduces Docsy as a submodule so that it will be easier to manage upgrades.

Details: this PR ...

  • Contributes to [Tracking] Upgrade Docsy #123
  • Eliminates the "hardcoded" files under website/themes/docsy, unlinked (via a git submodule or otherwise) to any Docsy repo
  • Eliminates the Bootstrap and FontAwesome (FA) submodules
  • Adds google/docsy@e67775a as a submodule, which itself has Bootstrap and FA as submodules. For the rationale, see below.
  • Updates NPM scripts to support building the website under this new setup
  • There is no effective change to the rendered site. For details, read below.

Preview: https://deploy-preview-160--tag-env-sustainability.netlify.app/


Context

The files under website/themes/docsy were a copy of google/docsy@f69a99d. But that commit is a fork commit based on google/docsy@e67775a from the official Docsy repo, with the following overrides:

Only in tmp/docsy: .git
Only in tmp/docsy: .gitmodules
Only in themes/docsy: .hugo_build.lock
diff --exclude=userguide --exclude=vendor -r themes/docsy/assets/scss/main.scss tmp/docsy/assets/scss/main.scss
36,37c36
<     overflow: auto;
<     
---
> 
diff --exclude=userguide --exclude=vendor -r themes/docsy/layouts/partials/footer.html tmp/docsy/layouts/partials/footer.html
5c5
<       <div class="col-6 col-sm-2 text-xs-center order-sm-2">
---
>       <div class="col-6 col-sm-4 text-xs-center order-sm-2">
12c12
<       <div class="col-6 col-sm-2 text-right text-xs-center order-sm-3">
---
>       <div class="col-6 col-sm-4 text-right text-xs-center order-sm-3">
19c19
<       <div class="col-12 col-sm-8 text-center py-2 order-sm-2">
---
>       <div class="col-12 col-sm-4 text-center py-2 order-sm-2">
23,25c23,24
<     {{ with .Site.GetPage "about" }}<p class="mt-2"><a href="{{ .RelPermalink }}">{{ .Title }}</a></p>{{ end }}
<   {{ end }}
<         {{ partial "hooks/footer-end.html" . }}
---
> 		{{ with .Site.GetPage "about" }}<p class="mt-2"><a href="{{ .RelPermalink }}">{{ .Title }}</a></p>{{ end }}
> 	{{ end }}
Only in themes/docsy/layouts/partials/hooks: footer-end.html

Regarding the diffs shown above:

Consequently I'm going to use the official Docsy commit google/docsy@e67775a as a basis for this repo's Docsy submodule.

No change to the rendered tag-env-sustainability site

If you build the site from this PR using the following command npm run serve:dev (which serves to disk), you'll see only one minor diff:

$ npm run serve:dev 
... # Terminate serve command once site has been generated to disk ^C
$ (cd public && git diff -- . ':(exclude)*.map')
... # Output shown below
$

Here's the diff output:

diff --git a/scss/main.css b/scss/main.css
index 2b42976..c0ea53c 100644
--- a/scss/main.css
+++ b/scss/main.css
@@ -14008,8 +14008,7 @@ div.toc-header {
     border-color: #403F4C; }
 
 footer {
-  min-height: 150px;
-  overflow: auto; }
+  min-height: 150px; }
   @media (max-width: 991.98px) {
     footer {
       min-height: 200px; } }

The override of footer's overflow is confirmed by examining CSS in dev tools when we examine the website prior to the changes in this PR:

Screen Shot 2023-08-07 at 17 52 23

Related

@aliok did a great job analyzing the situation for tag-contributor-strategy, though the situation was slightly different. For details, see cncf/tag-contributor-strategy#399.

/cc @nate-double-u @caniszczyk @cjyabraham

@netlify
Copy link

netlify bot commented Aug 7, 2023

Deploy Preview for tag-env-sustainability ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/tag-env-sustainability/deploys/64d19dfab9a0da161ad3c698
😎 Deploy Preview https://deploy-preview-160--tag-env-sustainability.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@chalin chalin force-pushed the chalin-im-docsy-submodule-2023-08-07 branch 2 times, most recently from 69f663c to 36a35cd Compare August 8, 2023 01:38
@chalin chalin force-pushed the chalin-im-docsy-submodule-2023-08-07 branch from 36a35cd to 55b17a3 Compare August 8, 2023 01:41
Copy link
Contributor

@cjyabraham cjyabraham left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@chalin chalin mentioned this pull request Aug 8, 2023
5 tasks
@chalin
Copy link
Collaborator Author

chalin commented Aug 8, 2023

Thanks for the review Chris. I'm unsure that the informal review policy is here, but with one approval I'm going merge and continue working on the Docsy upgrade. Comments are still welcome.

@chalin chalin merged commit 9c0fb1e into cncf:main Aug 8, 2023
9 checks passed
@chalin chalin deleted the chalin-im-docsy-submodule-2023-08-07 branch August 8, 2023 15:41
@chalin
Copy link
Collaborator Author

chalin commented Aug 8, 2023

For the record, both the original build of this PR and the build of the main branch (post merge) required that the cache be cleared (due to git submodule changes). I've done that in both cases, and the main build is green again:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants