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

Patch regression after #893 #904

Merged
merged 25 commits into from
Aug 2, 2023
Merged

Patch regression after #893 #904

merged 25 commits into from
Aug 2, 2023

Conversation

onweru
Copy link
Contributor

@onweru onweru commented Jun 15, 2023

Related issue

#893

Proposed Changes

This PR

  1. Applies this recommendation
  2. Fixes menu link alignment regression after Upgrade to docsy 0.7.0 #893
  3. Re-applies smooth scroll
  4. Fixes a markdown bug in the footer section

The changes here were applied on top of #893

Checklist

  • I have read the contributing guidelines.
  • I have inspected the website preview for accuracy.
  • [] I have signed off my issue.

deining and others added 13 commits June 8, 2023 23:33
Signed-off-by: Andreas Deininger <andreas@deininger.net>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

@deining would you kindly confirm that this PR incorporates the changes you recommended in your PR #893 to make the site more compatible with Docsy recommendations and future upgrades? Thanks very much.

onweru and others added 2 commits June 20, 2023 15:08
Signed-off-by: weru <fromweru@gmail.com>
@deining
Copy link
Contributor

deining commented Jun 20, 2023

@deining would you kindly confirm that this PR incorporates the changes you recommended in your PR #893

This PR looks good to me!
I do have two minor recommendations though:

  • hugo v0.114.0 was released just yesterday, so you can use this version for your deployment.
  • currently you are installing/using Go version 1.14.4 with your netlify deploy, I would recommend to go for latest released version 1.20.5.

Glad to see your site at docsy v0.7.0 now!

Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

Thanks, yes I see in the logs it's using Go 1.14. But that's not defined anywhere in the site nor is it in Netlify. Where is that coming from? In order to use Node 18 I had to set an environment variable in Netlify since we couldn't figure that out either. Any ideas or advice, @deining?

@deining
Copy link
Contributor

deining commented Jun 20, 2023

Thanks, yes I see in the logs it's using Go 1.14. But that's not defined anywhere in the site nor is it in Netlify. Where is that coming from? In order to use Node 18 I had to set an environment variable in Netlify since we couldn't figure that out either. Any ideas or advice, @deining?

https://docs.netlify.com/configure-builds/manage-dependencies/#go

You can set GO_VERSION as environment variable, too. Or, what you also can do:
In netlify.tomladd these lines:

GO_VERSION = 1.20.5
NODE_VERSION = 18.16.0

@chipzoller
Copy link
Contributor

Thanks, good suggestion. @onweru could you incorporate those changes here?

Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

@deining, I'm seeing that if I remove the env var from the Netlify side and add it here in netlify.toml that the deployment preview is once again failing. It's also not picking up the Go version even though it is specified. Any idea why that is?

Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

chipzoller commented Jun 20, 2023

Looks like the latest change did the trick!

3:39:09 PM: Installing Go version 1.20.5 (requested 1.20.5)
3:39:13 PM: go version go1.20.5 linux/amd64
3:39:13 PM: Using PHP version 8.0
3:39:13 PM: Installing Hugo 0.114.0
3:39:13 PM: hugo v0.114.0-9df2ec7988e5a217a14901cc76c0b7e76b2e9f02+extended linux/amd64 BuildDate=2023-06-19T17:01:43Z VendorInfo=gohugoio
3:39:14 PM: Started restoring cached Node.js version
3:39:15 PM: Finished restoring cached Node.js version
3:39:15 PM: v18.16.0 is already installed.
3:39:15 PM: Now using node v18.16.0 (npm v9.5.1)

@chipzoller
Copy link
Contributor

Sorry for the delay here, @onweru. I noticed that a patch to 0.7 was released. Would you mind bumping up to that so we get the fixes, please?

onweru and others added 2 commits June 29, 2023 10:46
Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

chipzoller commented Jul 12, 2023

Not that it's necessarily a bad thing, but did you notice that code blocks now are slightly different?

New

image

Old

image

In-line code formatting is also slightly different.

New

image

Old

image

@onweru
Copy link
Contributor Author

onweru commented Jul 12, 2023

@chipzoller, would you like me to restore the old format?

@chipzoller
Copy link
Contributor

Not necessarily, but what was the source or reason for the different styling? Is it something that could break in the future with Docsy upgrades potentially?

@chipzoller
Copy link
Contributor

chipzoller commented Jul 12, 2023

I do see a problem with how Mermaid diagrams are being rendered in these changes, however, and this would need to be addressed.

New

https://deploy-preview-904--kyverno.netlify.app/docs/installation/customization/#resource-filters

image

Old

https://kyverno.io/docs/installation/customization/#resource-filters

image

@onweru
Copy link
Contributor Author

onweru commented Jul 12, 2023

Not necessarily, but what was the source or reason for the different styling? Is it something that could break in the future with Docsy upgrades potentially?

The Docsy upgrade introduced styling changes. Future Docsy upgrades could contain more styling changes. We would have to evaluate if we like those changes or not.

@onweru
Copy link
Contributor Author

onweru commented Jul 12, 2023

I will fix the regressions.

onweru and others added 3 commits July 12, 2023 20:53
Signed-off-by: weru <fromweru@gmail.com>
Signed-off-by: weru <fromweru@gmail.com>
@chipzoller
Copy link
Contributor

This is somewhat of a more minor concern, but I am seeing code blocks with quite a bit more padding around the actual text. See https://deploy-preview-904--kyverno.netlify.app/docs/installation/uninstallation/#uninstall-kyverno-with-yaml

New

image

Old

image

@chipzoller
Copy link
Contributor

In https://deploy-preview-904--kyverno.netlify.app/docs/writing-policies/match-exclude/#match-statements have you noticed that the newer inline code styling doesn't seem to be applied to code inside tip boxes?

New

image

Old

image

@chipzoller
Copy link
Contributor

This is looking good so far 👍

@chipzoller
Copy link
Contributor

Just a couple things and I think we're good, @onweru.

Signed-off-by: weru <fromweru@gmail.com>
@chipzoller chipzoller self-requested a review August 2, 2023 21:40
Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

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

Thanks so much!

@chipzoller chipzoller merged commit bcf6ca7 into kyverno:main Aug 2, 2023
6 checks passed
@chipzoller chipzoller mentioned this pull request Aug 2, 2023
3 tasks
@aslafy-z aslafy-z mentioned this pull request Nov 28, 2023
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.

None yet

3 participants