-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
RHDEVDOCS-2796: Access-PipelineRun-TaskRun-Logs-in-OpenShift-Cluster-Logging #36023
RHDEVDOCS-2796: Access-PipelineRun-TaskRun-Logs-in-OpenShift-Cluster-Logging #36023
Conversation
✔️ Deploy Preview for osdocs ready! 🔨 Explore the source changes: 4e26503 🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/6151c648bc84de00071a849f 😎 Browse the preview: https://deploy-preview-36023--osdocs.netlify.app |
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.
Some nitpicks and a syntax fix added.
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
/lgtm |
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
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 left some comments for you to address and consider.
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
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.
Added some comments / suggestions
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.
Some additional comments
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Show resolved
Hide resolved
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.
Update: scratch that below comment, it renders fine now
One small issue with rendering also
@abrennan89 All suggestions incorporated, PTAL. |
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
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.
Some comments.
There seems to be a heading rendering oddly at the bottom of the page for additional resources but that might be due to the formatting.
https://deploy-preview-36023--osdocs.netlify.app/openshift-enterprise/latest/cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator#op-viewing-pipeline-logs-in-kibana_viewing-pipeline-logs-using-the-openshift-logging-operator
Also, the TOC is missing from this page. Please review https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assembly-file-metadata and ensure that you are following all the guidelines for assemblies.
@abrennan89 - Incorporated yesterday's suggestions. PTAL. |
cicd/pipelines/viewing-pipeline-logs-using-the-openshift-logging-operator.adoc
Outdated
Show resolved
Hide resolved
@abrennan89 - Everything is rendering correctly now. Thanks for the guidance. |
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.
/lgtm
Modified common-attributes to see if build error get resolved minor editorial fixes Minor editorial changes Trying to solve build errors Adding text before xref in Additional Resources Minor edits Incorporated Robert's comments Incorporated peer review comments Incorporated peer review editorial suggestions Incorporated peer review suggestions EDits to resolve rendering issue
New changes are detected. LGTM label has been removed. |
/cherrypick enterprise-4.7 |
/cherrypick enterprise-4.8 |
/cherrypick enterprise-4.9 |
@Preeticp: #36023 failed to apply on top of branch "enterprise-4.7":
In response to this:
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/test-infra repository. |
@Preeticp: new pull request created: #36792 In response to this:
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/test-infra repository. |
@Preeticp: new pull request created: #36793 In response to this:
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/test-infra repository. |
4.7
,4.8
,4.9
Note: Separate PR for cherrrypicking for
enterprise-4.7
to be created.