📖 add public api docs #1331

everettraven · 2024-10-03T14:02:25Z

Description

Adds documentation on the catalogd webserver that is considered as part of the public API
Adds documentation that explicitly calls out what our public API is
resolves What is our public API? #1239

Reviewer Checklist

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

netlify · 2024-10-03T14:02:42Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`c4e30d7`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/6700535763043f000892b90f
😎 Deploy Preview	https://deploy-preview-1331--olmv1.netlify.app
📱 Preview on mobile	Toggle 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 · 2024-10-03T14:16:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.37%. Comparing base (814bf63) to head (c4e30d7).
Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1331   +/-   ##
=======================================
  Coverage   76.37%   76.37%           
=======================================
  Files          41       41           
  Lines        2438     2438           
=======================================
  Hits         1862     1862           
  Misses        402      402           
  Partials      174      174

Flag	Coverage Δ
e2e	`58.44% <ø> (ø)`
unit	`53.60% <ø> (-0.05%)`	⬇️

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.

LalatenduMohanty

As per the last discussion HEAD https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/HEAD is also a supported method for catalog webserver

LalatenduMohanty · 2024-10-03T18:33:30Z

Apart from my above comment i.e. #1331 (review) everything else looks good.

everettraven · 2024-10-03T18:46:18Z

@LalatenduMohanty addressed

docs/refs/api/catalogd-webserver.md

michaelryanpeter

Small nits. I am going to look into getting some linting in the repo to address some of the consistency issues around terminology.

docs/refs/api/catalogd-webserver.md

michaelryanpeter · 2024-10-03T19:36:41Z

docs/refs/public-api.md

+    - [operator-controller API reference](./api/operator-controller-api-reference.md)
+    - [catalogd API reference](./api/catalogd-api-reference.md)
+- `catalogd` webserver. For more information on what this includes, see the [catalogd webserver documentation](./api/catalogd-webserver.md)


Since this section is pretty short, I think you could create an Additional resources section at the end of the page for the links. Maybe after line 12?

## Additional resources - Operator Controller API reference - Catalogd API reference - Catalogd web server

Would you mind elaborating on what value this would bring to a reader over the current setup?

To me, the links are a little distracting where they are. The point you are trying to get across is to define the scope of the public API. I think that if you break out the links to the API references at the end of the page, it is a little tidier and your intended meaning is more direct.

That said, this is 100% a nit. Feel free to ignore it doesn't feel relevant to you.

As a heuristic for determining whether something should be an inline link vs an additional resource, I ask myself if the reader needs to stop and read the link before continuing.

If the answer is yes, then I use an inline link. Typically, this would be in the case of a prerequisite or context information that is vital to understanding a concept or completing a task.

If the reader can follow up on the information later or if the link is provided for helpful context, then I break out the link in an additional resources section.

As long as the content is OK - i'd like to get this merged in so we at least have it and can keep progressing towards our v1.0.0 release. We can always tweak this in the future!

Signed-off-by: everettraven <everettraven@gmail.com>

trgeiger · 2024-10-04T14:03:51Z

docs/api-reference/catalogd-webserver.md

+# Catalogd web server
+
+[Catalogd](https://github.com/operator-framework/catalogd), the OLM v1 component for making catalog contents available on cluster, includes
+a web server that serves catalog contents to clients via a HTTP(S) endpoint.


This would be "an HTTP(s)" since HTTP is pronounced with a starting vowel sound. You determine the article via the acronym pronunciation.

Should be taken care of in 647a9f7

trgeiger · 2024-10-04T14:06:16Z

docs/api-reference/catalogd-webserver.md

+
+### Compression Support
+
+The `catalogd` web server supports gzip compression of responses, which can significantly reduce associated network traffic.  In order to signal to that the client handles compressed responses, the client must include `Accept-Encoding: gzip` as a header in the HTTP request.


"to signal that the client"

Should be taken care of in 647a9f7

michaelryanpeter · 2024-10-04T14:42:06Z

+1 to Tayler's comments. Otherwise, lgtm.

Signed-off-by: everettraven <everettraven@gmail.com>

docs/api-reference/catalogd-webserver.md

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>

everettraven requested a review from a team as a code owner October 3, 2024 14:02

LalatenduMohanty requested changes Oct 3, 2024

View reviewed changes

everettraven requested a review from LalatenduMohanty October 3, 2024 18:46

everettraven enabled auto-merge October 3, 2024 18:58

grokspawn reviewed Oct 3, 2024

View reviewed changes

docs/refs/api/catalogd-webserver.md Outdated Show resolved Hide resolved

michaelryanpeter reviewed Oct 3, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2024

everettraven added 2 commits October 3, 2024 16:09

public api docs

3b934d9

Signed-off-by: everettraven <everettraven@gmail.com>

post rebase fixes

fc29827

Signed-off-by: everettraven <everettraven@gmail.com>

everettraven force-pushed the docs/public-api branch from de1767e to fc29827 Compare October 3, 2024 20:12

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2024

trgeiger reviewed Oct 4, 2024

View reviewed changes

address nits

647a9f7

Signed-off-by: everettraven <everettraven@gmail.com>

joelanford reviewed Oct 4, 2024

View reviewed changes

docs/api-reference/catalogd-webserver.md Outdated Show resolved Hide resolved

Update docs/api-reference/catalogd-webserver.md

c4e30d7

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>

LalatenduMohanty approved these changes Oct 4, 2024

View reviewed changes

joelanford approved these changes Oct 4, 2024

View reviewed changes

everettraven added this pull request to the merge queue Oct 4, 2024

Merged via the queue into operator-framework:main with commit eb64d98 Oct 4, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

📖 add public api docs #1331

📖 add public api docs #1331

everettraven commented Oct 3, 2024

netlify bot commented Oct 3, 2024 •

edited

Loading

codecov bot commented Oct 3, 2024 •

edited

Loading

LalatenduMohanty left a comment

LalatenduMohanty commented Oct 3, 2024 •

edited

Loading

everettraven commented Oct 3, 2024

michaelryanpeter left a comment

michaelryanpeter Oct 3, 2024

everettraven Oct 3, 2024

michaelryanpeter Oct 3, 2024 •

edited

Loading

michaelryanpeter Oct 3, 2024 •

edited

Loading

everettraven Oct 4, 2024

trgeiger Oct 4, 2024

everettraven Oct 4, 2024

trgeiger Oct 4, 2024

everettraven Oct 4, 2024

michaelryanpeter commented Oct 4, 2024


		### Compression Support

		The `catalogd` web server supports gzip compression of responses, which can significantly reduce associated network traffic. In order to signal to that the client handles compressed responses, the client must include `Accept-Encoding: gzip` as a header in the HTTP request.

📖 add public api docs #1331

📖 add public api docs #1331

Conversation

everettraven commented Oct 3, 2024

Description

Reviewer Checklist

netlify bot commented Oct 3, 2024 • edited Loading

✅ Deploy Preview for olmv1 ready!

codecov bot commented Oct 3, 2024 • edited Loading

Codecov Report

LalatenduMohanty left a comment

Choose a reason for hiding this comment

LalatenduMohanty commented Oct 3, 2024 • edited Loading

everettraven commented Oct 3, 2024

michaelryanpeter left a comment

Choose a reason for hiding this comment

michaelryanpeter Oct 3, 2024

Choose a reason for hiding this comment

everettraven Oct 3, 2024

Choose a reason for hiding this comment

michaelryanpeter Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

michaelryanpeter Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

everettraven Oct 4, 2024

Choose a reason for hiding this comment

trgeiger Oct 4, 2024

Choose a reason for hiding this comment

everettraven Oct 4, 2024

Choose a reason for hiding this comment

trgeiger Oct 4, 2024

Choose a reason for hiding this comment

everettraven Oct 4, 2024

Choose a reason for hiding this comment

michaelryanpeter commented Oct 4, 2024

netlify bot commented Oct 3, 2024 •

edited

Loading

codecov bot commented Oct 3, 2024 •

edited

Loading

LalatenduMohanty commented Oct 3, 2024 •

edited

Loading

michaelryanpeter Oct 3, 2024 •

edited

Loading

michaelryanpeter Oct 3, 2024 •

edited

Loading