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

📖 Update documentation for "Finding extensions to install" #696

Conversation

michaelryanpeter
Copy link
Contributor

@michaelryanpeter michaelryanpeter commented Mar 13, 2024

Description

Preview links

Reviewer Checklist

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

@michaelryanpeter michaelryanpeter requested a review from a team as a code owner March 13, 2024 21:15
Copy link

netlify bot commented Mar 13, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit b87ba44
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/65f21769697cc400086e98d9
😎 Deploy Preview https://deploy-preview-696--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.

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.01%. Comparing base (8cdb7c3) to head (b87ba44).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #696   +/-   ##
=======================================
  Coverage   64.01%   64.01%           
=======================================
  Files          22       22           
  Lines        1370     1370           
=======================================
  Hits          877      877           
  Misses        442      442           
  Partials       51       51           
Flag Coverage Δ
e2e 47.37% <ø> (ø)
unit 58.41% <ø> (ø)

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.

## Package queries

Available packages in a catalog
: `jq -s '.[] | select( .schema == "olm.package")`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For all of the jq commands shown here can we use the "code fence" syntax (i.e triple backtick)? This results in a user being able to easily copy the command to their clipboard via a handy dandy button and IMO presents a nicer docs experience when used where possible.

Copy link
Contributor Author

@michaelryanpeter michaelryanpeter Mar 14, 2024

Choose a reason for hiding this comment

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

I created a follow up issue: #698

Basically, I can't get the code blocks to render in the definition list. I think I might need to move the list to a table, but if I get the code block to work, the table gets stretched really wide. There is probably something really small that I am missing with mkdocs' bespoke syntax, but after an hour trying to figure it out yesterday, I thought I would try this temporary hack.

In the interest of making the Kubecon deadline, I would prefer to get the rest of the content in place and circle back after. WDYT, @everettraven?

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM - I don't think this nit should at all block this from merging!

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

One minor nit that I don't think should hold this PR. Otherwise looks good to me!

Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2024
@everettraven everettraven added this pull request to the merge queue Mar 14, 2024
Merged via the queue into operator-framework:main with commit 1ef2792 Mar 14, 2024
20 checks passed
After you add a catalog of exetensions to your cluster, you must port forward your catalog as a service.

Choose a reason for hiding this comment

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

typo exetensions should be extensions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TY! Addressed in #728

## Procedure

1. Port forward the catalog server sevice:

Choose a reason for hiding this comment

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

sevice should be service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TY! Addressed in #728

```

??? success

Choose a reason for hiding this comment

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

what does ??? provide?

Copy link
Contributor Author

@michaelryanpeter michaelryanpeter Apr 1, 2024

Choose a reason for hiding this comment

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

Sorry, I had a draft for this comment, but I forgot to submit it.

The ??? creates a code block with a toggle.

I find it especially useful for long lists of example output after you run a command.

image

image

michaelryanpeter added a commit to michaelryanpeter/operator-controller that referenced this pull request Apr 1, 2024
@michaelryanpeter michaelryanpeter mentioned this pull request Apr 1, 2024
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Apr 1, 2024
* Fix catalog queries code blocks

Improve the formatting of the code blocks in the catalog queries
description list

* Fix typos

Fix typos called out in
#696 (comment)
and
#696 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants