Skip to content

Conversation

prithvipatil97
Copy link
Contributor

@prithvipatil97 prithvipatil97 commented Feb 3, 2025

Version(s):

RHOCP-4.18, RHOCP-4.17, RHOCP-4.16, RHOCP-4.15, RHOCP-4.14

Issue:

https://issues.redhat.com/browse/OBSDOCS-1346

Link to docs preview:

https://87991--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/logging/logging-6.0/log6x-about.html

QE review:

  • QE has approved this change.

Additional information:

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 3, 2025
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Feb 3, 2025

🤖 Tue Feb 04 19:28:57 - Prow CI generated the docs preview:

https://87991--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/logging/logging-6.0/log6x-about.html

* Cluster administrator permissions
* You have administrator permissions.
* You installed the OpenShift CLI (oc).
* You have access to a supported object store. For example: AWS S3, Google Cloud Storage, Azure, Swift, Minio, or OpenShift Data Foundation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤖 [error] OpenShiftAsciiDoc.SuggestAttribute: Use the AsciiDoc attribute '{rh-storage}' rather than the plain text product term 'OpenShift Data Foundation', unless your use case is an exception.

@prithvipatil97
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Feb 3, 2025
@eromanova97
Copy link
Contributor

/remove-label peer-review-needed

/label peer-review-in-progress

@openshift-ci openshift-ci bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Feb 4, 2025
Copy link
Contributor

@eromanova97 eromanova97 left a comment

Choose a reason for hiding this comment

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

I left some small comments, otherwise looks good to me!

However, as I am new to peer review squad, please wait for @dfitzmau to review again and for the peer-review-done label before proceeding. Thank you!

Comment on lines -98 to +100
. Install the Cluster Observability Operator.
. Install the link:https://docs.openshift.com/container-platform/4.16/observability/cluster_observability_operator/installing-the-cluster-observability-operator.html#installing-the-cluster-observability-operator-in-the-web-console-_installing_the_cluster_observability_operator[Cluster Observability Operator].
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you use link here instead of xref (to avoid xrefs in a module), however, I am not sure if this is the best practice, even more with the decommisioning of docs.openshift (which will probably break these links).

I would consider leaving this as it was and instead adding an additional resources section after the module in an assembly, where you would link this correctly with xref

[role="_additional-resources"]
.Additional resources
* xref:../

Copy link
Contributor

@eromanova97 eromanova97 Feb 4, 2025

Choose a reason for hiding this comment

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

Also, the Cluster Observability Operator has an attribute you can use here: {coo-full}

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, @eromanova97 . Excellent review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @eromanova97 ,
Thanks for your attention here in peer review.

I apologize for the inconvenience caused.
I understand the suggested changes.

Thank you again,

Regard,
Prithviraj Patil

- Here is the link:
https://docs.openshift.com/container-platform/4.16/observability/logging/logging-6.0/log6x-about.html#quick-start

- Problems:
  - Quick Start indentation is wrong.
  - Prerequisites are missing.
  - Adding link to Step 6.

- We are performing the following changes through this PR:
  - Corrected Quick Start indentation, it should be in line with Validation.
  - Added required Prerequisites.
  - Added installation guideline link in the Step 6. Install the Cluster Observability Operator.

Update observability/logging/logging-6.0/log6x-about.adoc

Committing suggested changes.

Co-authored-by: Eliska Romanova <eromanov@redhat.com>

Update  (commit-2) observability/logging/logging-6.0/log6x-about.adoc

Performing suggested changes in Prerequisites

Co-authored-by: Eliska Romanova <eromanov@redhat.com>
Copy link

openshift-ci bot commented Feb 4, 2025

@prithvipatil97: 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.

@prithvipatil97
Copy link
Contributor Author

Hello @eromanova97 ,
I have successfully committed first change:

This is a patch-29:

Here is the push command output for both the commit:

[pripatil@supportshell-1 openshift-docs]$ git push origin HEAD --force
Username for 'https://github.com': prithvipatil97
Password for 'https://prithvipatil97@github.com': 
Enumerating objects: 11, done.
Counting objects: 100% (11/11), done.
Delta compression using up to 32 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 1.19 KiB | 1.19 MiB/s, done.
Total 6 (delta 5), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (5/5), completed with 5 local objects.
To https://github.com/prithvipatil97/openshift-docs
 + 1cc483517e...1358e1d3d7 HEAD -> patch-29 (forced update)         <<===
[pripatil@supportshell-1 openshift-docs]$ git push origin HEAD --force
Username for 'https://github.com': prithvipatil97
Password for 'https://prithvipatil97@github.com': 
Enumerating objects: 11, done.
Counting objects: 100% (11/11), done.
Delta compression using up to 32 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 1.21 KiB | 1.21 MiB/s, done.
Total 6 (delta 5), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (5/5), completed with 5 local objects.
To https://github.com/prithvipatil97/openshift-docs
 + 3edb34a4f6...f9484e27cf HEAD -> patch-29 (forced update)       <<===

Kindly have a look and let me know, if it is correct now.
Also, please confirm if we are good to proceed further.

Regards,
Prithviraj Patil

@prithvipatil97
Copy link
Contributor Author

Hello Team,
Thanks for your help here.
I have committed the suggested changes.
But we found some other points to be changed from same documentation.
Hence I am closing this PR and we will continue with new PR with all the suggested points.
Here is the new PR Link: #88424

Thanks again.

Regards,
Prithviraj Patil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

peer-review-in-progress Signifies that the peer review team is reviewing this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants