-
Notifications
You must be signed in to change notification settings - Fork 2.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
[3.2] Direct patch for the Data source and Logging guide #37394
[3.2] Direct patch for the Data source and Logging guide #37394
Conversation
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
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
docs/src/main/asciidoc/logging.adoc
Outdated
@@ -9,7 +9,7 @@ include::_attributes.adoc[] | |||
:categories: core,getting-started,observability | |||
:diataxis-type: reference | |||
|
|||
Read about the use of logging API in Quarkus, configuring logging output, and using logging adapters to unify the output from other logging APIs. | |||
Read about the use of the JBoss Logging API in Quarkus, configuring logging output, and using logging adapters to unify the output from other logging APIs. |
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.
A few lines below we mention that developers can use one of many APIs... so I don't think this change makes sense
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.
Feel free to merge though, I'm not blocking the PR.
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.
Ok. Good point. I will patch it like this:
Read about the use of logging APIs in Quarkus, configuring logging output, and using logging adapters for unified output.
Thank you, @yrodiere
Cheers!
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'm the one who suggested this edit, and I felt it was fine because other logging APIs are mentioned further in the sentence. But the fix seems good too.
Signed-off-by: Michal Maléř <mmaler@redhat.com>
5fca918
to
9b680a4
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.
Hi @MichalMaler. All looks good to me!
🙈 The PR is closed and the preview is expired. |
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
Removing the
in Quarkus
part from theData source
guide. Asked some SMEs, it is not needed there.Then, I tweaked slightly the Data source and Logging guide abstracts so that we can perform some RHBQ name postprocesing and they contain the proper grammar and information.