-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Netobserv API doc regeneration #76593
Conversation
🤖 Mon Jun 10 20:58:46 - Prow CI generated the docs preview: |
@memodi @jotak can you PTAL at this PR for API doc regeneration? Joel, I left some review feedback in your PR: netobserv/network-observability-operator#657 (review). So I guess once that's incorporated, I'll copy it over here. It was mostly minor copy edits, things like capitalization and backticks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good overall, just one comment.
45317dc
to
03336dc
Compare
/retest |
03336dc
to
85ec798
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that some of the things I commented on were previously accepted text, so I think it's OK to ignore those comments. (I stopped after I realized, about halfway in.) But I think that the link:
issue might need to be fixed, just in case it might cause issues in rendering. Also I've been told that blank/empty lines can cause issues, although they seem to be fine in the preview.
modules/network-observability-flowcollector-api-specifications.adoc
Outdated
Show resolved
Hide resolved
modules/network-observability-flowcollector-api-specifications.adoc
Outdated
Show resolved
Hide resolved
modules/network-observability-flowcollector-api-specifications.adoc
Outdated
Show resolved
Hide resolved
modules/network-observability-flowcollector-api-specifications.adoc
Outdated
Show resolved
Hide resolved
modules/network-observability-flowcollector-api-specifications.adoc
Outdated
Show resolved
Hide resolved
@kcarmichael08 thanks for the review. I know this is a lot to get through. @jotak do you want to take this feedback upstream with the |
thanks @kcarmichael08 - almost everything should be addressed upstream in netobserv/network-observability-operator#672 - except a few things which I commented here. |
85ec798
to
28bfcd2
Compare
28bfcd2
to
42deab0
Compare
@skrthomas: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @skrthomas !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/label qe-approved
Version(s):
Merge to only the no-1.6 branch - no cherrypicks are required.
This PR is part of an experiment for simplifying merges for asynchronous content, and I will open one PR against main to incorporate all of the Network Observability 1.6 content just before its GA
Issue:
Link to docs preview:
https://76593--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/network_observability/flowcollector-api.html
QE review:
Additional information:
Notes to peer reviewers:
spec...advanced.scheduling
that contain the same specs fortolerations
,priorityClassName
,nodeSelector
, and affinity. After you look at one, feel free to glaze over the others :)