MINOR: Improve Connect docs by mimaison · Pull Request #11642 · apache/kafka

mimaison · 2022-01-03T19:12:14Z

Fix indendation of code blocks
Add links to all SMTs and Predicates

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

- Fix indendation of code blocks - Add links to all SMTs and Predicates

showuon

@mimaison , thanks for fixing it. It will make the doc look better! Left a comment. Thanks.

showuon · 2022-01-04T05:55:05Z

    <pre class="brush: bash;">
-    &gt; bin/connect-standalone.sh config/connect-standalone.properties connector1.properties [connector2.properties ...]
+&gt; bin/connect-standalone.sh config/connect-standalone.properties connector1.properties [connector2.properties ...]
    </pre>


I found we put an additional line for each command.

We should put the ending </pre> at the end of the command, ex:

<pre class="brush: bash;"> > bin/connect-standalone.sh config/connect-standalone.properties connector1.properties [connector2.properties ...]</pre>

Same as other commands.

Good point! I've pushed an update

showuon

Thanks for the update. The only thing I'm not sure is that if the SMT and predicate link is correct or not. Other than that, LGTM! Thank you.

mimaison · 2022-01-25T11:38:53Z

@dajac can you take a look? Thanks

tombentley · 2022-02-09T14:52:21Z

-        return configs;
-    }
-    </pre>
+@Override


Does putting this in a CDATA let us avoid the eye-bleeding HTML escaping?

I'm not familiar with CDATA but from a quick search it seems it shouldn't be used in HTML documents.
https://developer.mozilla.org/en-US/docs/Web/API/CDATASection states:

Note: CDATA sections should not be used within HTML they are considered as comments and not displayed.

tombentley · 2022-02-09T14:55:02Z


        out.print("<h5>");
-        out.print(docInfo.predicateName);
+        out.print("<a href=\"#" + docInfo.predicateName + "\">" + docInfo.predicateName + "</a>");


Are this really href not name? If so what are they linking to?

It's linking to the existing <div id="..."> just above on line 61

Could you let me know where does these predicateName appear in the doc?
here? https://kafka.apache.org/documentation/#connect_predicates
And what will be the expected output? What hyperlink destination we expected to link to?

Thank you.

Predicates are listed in this section: https://kafka.apache.org/documentation/#connect_predicates

Currently the names of predicates are listed but they are not links so we can't easily copy direct links to them.

OK, I got it. You want to add an anchor point in each predicate and transformation. I see now. I've checked, there is already the HTML element id = predicate names and transformation names, so adding an anchor link with the name can link to the position. Looks good to me. Thanks for the explanation.

mimaison · 2022-02-21T11:23:22Z

@showuon @tombentley Can you take another look? Thanks

showuon

@mimaison , for the html changes, I think they are good to be merged. But for the doc tool part, I am still not sure if the change is good or not since I don't know how these tool work. Might need your help to explain to me.

Or, maybe we can first merge the html changes, and discuss the tools change in a separate PR. Up to you. Thanks.

showuon · 2022-02-22T03:15:34Z


        out.print("<h5>");
-        out.print(docInfo.predicateName);
+        out.print("<a href=\"#" + docInfo.predicateName + "\">" + docInfo.predicateName + "</a>");


Could you let me know where does these predicateName appear in the doc?
here? https://kafka.apache.org/documentation/#connect_predicates
And what will be the expected output? What hyperlink destination we expected to link to?

Thank you.

showuon

LGTM! Thanks for the patch!

showuon · 2022-02-22T12:47:31Z


        out.print("<h5>");
-        out.print(docInfo.predicateName);
+        out.print("<a href=\"#" + docInfo.predicateName + "\">" + docInfo.predicateName + "</a>");


OK, I got it. You want to add an anchor point in each predicate and transformation. I see now. I've checked, there is already the HTML element id = predicate names and transformation names, so adding an anchor link with the name can link to the position. Looks good to me. Thanks for the explanation.

showuon · 2022-02-22T23:35:45Z

@tombentley , sorry, I found I forgot to ask you if you have other comments. If you have any comments, please raise it. Sorry!

* apache-kafka/trunk: (49 commits) KAFKA-12738: send LeaveGroup request when thread dies to optimize replacement time (apache#11801) MINOR: Skip fsync on parent directory to start Kafka on ZOS (apache#11793) KAFKA-12738: track processing errors and implement constant-time task backoff (apache#11787) MINOR: Cleanup admin creation logic in integration tests (apache#11790) KAFKA-10199: Add interface for state updater (apache#11499) KAFKA-10000: Utils methods for overriding user-supplied properties and dealing with Enum types (apache#11774) KAFKA-10000: Add new metrics for source task transactions (apache#11772) KAFKA-13676: Commit successfully processed tasks on error (apache#11791) KAFKA-13511: Add support for different unix precisions in TimestampConverter SMT (apache#11575) MINOR: Improve Connect docs (apache#11642) ...

MINOR: Improve Connect docs

e65c697

- Fix indendation of code blocks - Add links to all SMTs and Predicates

showuon reviewed Jan 4, 2022

View reviewed changes

Inline </pre> tags to avoid extra empty lines

65f3394

showuon reviewed Jan 4, 2022

View reviewed changes

tombentley reviewed Feb 9, 2022

View reviewed changes

showuon reviewed Feb 22, 2022

View reviewed changes

showuon approved these changes Feb 22, 2022

View reviewed changes

showuon merged commit 576496a into apache:trunk Feb 22, 2022

mimaison deleted the connect_docs branch February 22, 2022 13:33

Conversation

mimaison commented Jan 3, 2022

Committer Checklist (excluded from commit message)

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

mimaison commented Jan 25, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimaison Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

showuon Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimaison commented Feb 21, 2022

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

showuon Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

showuon commented Feb 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mimaison Feb 9, 2022 •

edited

Loading

showuon Feb 22, 2022 •

edited

Loading

showuon Feb 22, 2022 •

edited

Loading