Skip to content

📖 Improve formatting on the docs website - #1446

Merged
m1kola merged 1 commit into
operator-framework:mainfrom
m1kola:fix_docs_rendering
Nov 13, 2024
Merged

📖 Improve formatting on the docs website#1446
m1kola merged 1 commit into
operator-framework:mainfrom
m1kola:fix_docs_rendering

Conversation

@m1kola

@m1kolam1kola commented Nov 12, 2024

Copy link
Copy Markdown
Member

Description

Fixes a number of formatting/rendering issues. Mostly around lists, admonitions (notes, warnings, etc) and collapsible text.

Tip

Recommend reviewing the changes with git diff --ignore-all-space (-w) option. GitHub UI also has this option.
Screenshot 2024-11-12 at 15 59 15

Tip

We also have docs deployment preview. See the comment from netlify bot below.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@openshift-ciopenshift-ciBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 12, 2024
@netlify

netlifyBot commented Nov 12, 2024

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commitc916dc7
🔍 Latest deploy loghttps://app.netlify.com/sites/olmv1/deploys/6734a3daf64a1b000848b566
😎 Deploy Previewhttps://deploy-preview-1446--olmv1.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.

Comment on lines -54 to +56
** Disable CRD Upgrade Safety Check:**
**Command Example:**

**Patch the ClusterExtension Resource:**
If you prefer using the command line, you can use `kubectl` to modify the upgrade CRD safety check configuration.

@m1kolam1kolaNov 12, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This is the only text related change because it was very odd to read this.

Everything else is just formatting.


Screenshot 2024-11-12 at 16 03 13

VS

Screenshot 2024-11-12 at 16 03 17

@m1kola
m1kola marked this pull request as ready for review November 12, 2024 15:04
@m1kola
m1kola requested a review from a team as a code ownerNovember 12, 2024 15:04
@openshift-ciopenshift-ciBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 12, 2024
@codecov

codecovBot commented Nov 12, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.91%. Comparing base (046c3df) to head (9f7db18).
Report is 1 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #1446 +/- ##
=======================================
Coverage 74.91% 74.91% =======================================
Files 42 42 Lines 3241 3241 =======================================
Hits 2428 2428 Misses 642 642 Partials 171 171 
FlagCoverage Δ
e2e51.95% <ø> (-0.10%)⬇️
unit57.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines -66 to +69
> [!NOTE]
> Currently, OLM 1.0 does not support automatic upgrades to the next major version. You must manually verify and perform major version upgrades. For more information about major version upgrades, see [Manually verified upgrades and downgrades](#manually-verified-upgrades-and-downgrades).
!!! note
Currently, OLM 1.0 does not support automatic upgrades to the next major version. You must manually verify and perform major version upgrades. For more information about major version upgrades, see [Manually verified upgrades and downgrades](#manually-verified-upgrades-and-downgrades).

@m1kolam1kolaNov 12, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

!!! note This renders well in our mkdocs website, but not on GitHub. [!NOTE] renders well on GitHub, but not in mkdocs.

Same applies to other admonitions.

Ref: https://squidfunk.github.io/mkdocs-material/reference/admonitions/#classic-admonitions-docsstylesheetsextracss

We are changing these admonitions in docs dir because these are docs which will be rendered on the website. Leaving everything outside of docs dir as is - these will likely be viewed via GitHub UI.

Comment threadCONTRIBUTING.md
kind create cluster operator-controller
```
```sh
kind create cluster operator-controller

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It was rendering fine in GH UI. However this doc is also used in the website. This formatting works fine in both GH and on the website.

Comment threaddocs/concepts/upgrade-support.md

@camilamacedo86camilamacedo86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assuming that you finished (since I cannot see in the title WIP)

Hi @m1kola

The emoji here is 📖 not 🌱

🌱 is only for things that does not bring impacted to the end users at all (i.e changes in the e2e tests)

So could you please fix the emoji?

Otherwise,

I gave a look on this one with the preview and all shows fine 🥇
Great work !!! Well done

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2024
@m1kolam1kola changed the title 🌱 Improve formatting on the docs website📖 Improve formatting on the docs websiteNov 12, 2024
@m1kola

Copy link
Copy Markdown
MemberAuthor

Forgot we have a separate emoji for docs. Updated.

@trgeigertrgeiger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting a few small changes

Comment threaddocs/concepts/upgrade-support.md
Comment threaddocs/contribute/developer.md
Comment threaddocs/howto/derive-service-account.md
Comment threaddocs/tutorials/upgrade-extension.md Outdated
@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2024
@m1kola
m1kola requested a review from trgeigerNovember 12, 2024 16:27
@m1kola

Copy link
Copy Markdown
MemberAuthor

@trgeiger lovely stuff! Please take a look at the latest push: it addresses your feedback + I noticed a broken list under "Putting it all together" in docs/howto/derive-service-account.md

@openshift-ci

Copy link
Copy Markdown

@trgeiger: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

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-sigs/prow repository.

@trgeiger

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2024
@michaelryanpeter

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci

Copy link
Copy Markdown

@michaelryanpeter: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

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-sigs/prow repository.

@camilamacedo86

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2024
@m1kola

Copy link
Copy Markdown
MemberAuthor

Had to rebase due to #1228

@m1kola

Copy link
Copy Markdown
MemberAuthor

Blocked by #1451. Will require another rebase once it merges.

@camilamacedo86camilamacedo86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2024
Fixes a number of formatting/rendering issues. Mostly around
lists, admonitions (notes, warnings, etc) and collapsible text.
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2024
@openshift-ci

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@m1kola

Copy link
Copy Markdown
MemberAuthor

Rebased on top of #1451 so it can pass upgrade-e2e.

@m1kola
m1kola enabled auto-merge November 13, 2024 13:13

@perdasilvaperdasilva left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome! Thank you!!!

@m1kola
m1kola added this pull request to the merge queue Nov 13, 2024
Merged via the queue into operator-framework:main with commit ac75d65Nov 13, 2024
@m1kola
m1kola deleted the fix_docs_rendering branch November 13, 2024 14:03
Sign up for freeto 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.

5 participants

@m1kola@trgeiger@michaelryanpeter@camilamacedo86@perdasilva