Skip to content

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
@netlify
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.

@codecov
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
```

**This CR is most helpful when exploring the versions of a package that are available for installation on cluster, and the upgrade graph of versions** (eg if v0.5.0 of `argocd-operator` is installed on cluster, what is the next upgrade available? The answer is v0.6.0).
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

$ curl http://localhost:8080/catalogs/operatorhubio/all.json | jq -s '.[] | select(.schema == "olm.package") | .name'
```
??? 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