fix: update readme to have proper wordings #9

Joel-hanson · 2025-01-02T06:42:15Z

README been reviewed or updated in light of what we are adding to the converter and transformation documentation cards

Description

README been updated in light of what we are adding to the converter and transformation documenation cards

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

E2E
Unit Test
Integration Test

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

- README been reviewed or updated in light of what we are adding to the converter and transformation documentation cards Signed-off-by: Joel Hanson <[email protected]>

gabortpubs

Added suggestions.

gabortpubs · 2025-01-07T13:21:49Z

README.md

+| `xsd.schema.path`     |                   | Location of a schema file to use to parse the XML string.                                 |
 | `xml.doc.flat.enable` | `false`           | Set to `true` if the XML strings contain a single value (e.g. `<root>the message</root>`) |

 Optional configuration that can be set when using the plugin to create XML strings from Connect records (Conect Record -> XML string)


Are we missing a table from here? If so, the lead-in sentence needs to be updated as well.

README.md

Co-authored-by: gabortpubs <[email protected]> Signed-off-by: Joel Hanson <[email protected]>

README.md

gabortpubs

@Joel-hanson Added a few more comments.

gabortpubs · 2025-01-08T10:12:49Z

README.md

 | `xml.doc.flat.enable` | `false`           | Set to `true` if the XML strings contain a single value (e.g. `<root>the message</root>`) |

-Optional configuration that can be set when using the plugin to create XML strings from Connect records (Conect Record -> XML string)
+The following table lists optional configuration that can be set when turning XML strings from Connect records by using the plug-ins (Connect Record to XML string)


I think this sentence is not quite right, as it states "turning XML strings from Connect records" but misses the bit about "into what".

Also, there's an empty table below which I assume should have some content.

@gabortpubs there is a table below this sentence, am I missing or misunderstanding something here.

The following is the table that we are referencing here. https://github.com/ibm-messaging/kafka-connect-xml-converter/pull/9/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5L26-L28

Ok, it wasn't showing the whole file in your PR, and I forgot to click "expand", so I guess this is user error on my part.

gabortpubs · 2025-01-08T10:13:31Z

README.md

-| `xsd.schema.path`     |                   | Location of a schema file to use to parse the XML string.                               |
+| **Option**            | **Default value** | **Notes**                                                                                 |
+| --------------------- | ----------------- | ----------------------------------------------------------------------------------------- |
+| `root.element.name`   | `root`            | The name of the root element in the XML document being parsed.                            |


gabortpubs · 2025-01-08T10:14:04Z

README.md

+| **Option**            | **Default value** | **Notes**                                                                                 |
+| --------------------- | ----------------- | ----------------------------------------------------------------------------------------- |
+| `root.element.name`   | `root`            | The name of the root element in the XML document being parsed.                            |
+| `xsd.schema.path`     |                   | Location of a schema file to use to parse the XML string.                                 |


gabortpubs · 2025-01-08T10:14:20Z

README.md

+| --------------------- | ----------------- | ----------------------------------------------------------------------------------------- |
+| `root.element.name`   | `root`            | The name of the root element in the XML document being parsed.                            |
+| `xsd.schema.path`     |                   | Location of a schema file to use to parse the XML string.                                 |
 | `xml.doc.flat.enable` | `false`           | Set to `true` if the XML strings contain a single value (e.g. `<root>the message</root>`) |


dalelane · 2025-01-13T10:22:10Z

README.md

+  A Single Message Transform (SMT) that takes a Kafka Connect record containing an XML string and transforms it into a structured Connect record.
 - `com.ibm.eventstreams.kafkaconnect.plugins.xml.XmlMQRecordBuilder`
-    - an MQ Source Record builder for parsing MQ messages containing XML strings
+  - An MQ Source Record builder for parsing MQ messages containing XML strings.


minor formatting inconsistency - the first two are single-line whereas the third is an indented bullet

Please make all three consistent

dalelane · 2025-01-13T10:29:13Z

README.md


 - `com.ibm.eventstreams.kafkaconnect.plugins.xml.XmlConverter`
-    - a Kafka Connect converter for converting to/from XML strings
+  A Kafka Connect converter for converting between Kafka Connect’s internal data format and XML strings.


Suggested change

A Kafka Connect converter for converting between Kafka Connect’s internal data format and XML strings.

A Kafka Connect converter for converting between structured objects (Kafka Connect’s internal data format & Java Maps and Lists) and XML strings.

@dalelane Minor thing, but can we change the part in brackets to avoid using the ampersand?

(Kafka Connect’s internal data format, and Java Maps and Lists)

fine with me - main thing to pick up was the converter doesn't only support Struct, it works with Map and List as well, but grammar/punctuation-wise I'm agnostic :-)

Signed-off-by: Joel Hanson <[email protected]>

fix: update readme to have proper wordings

9e9bc40

- README been reviewed or updated in light of what we are adding to the converter and transformation documentation cards Signed-off-by: Joel Hanson <[email protected]>

gabortpubs reviewed Jan 7, 2025

View reviewed changes

fix: Apply suggestions from code review

0ce4c59

Co-authored-by: gabortpubs <[email protected]> Signed-off-by: Joel Hanson <[email protected]>

Joel-hanson commented Jan 8, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

Joel-hanson requested a review from gabortpubs January 8, 2025 09:21

gabortpubs reviewed Jan 8, 2025

View reviewed changes

dalelane self-requested a review January 8, 2025 15:27

Joel-hanson force-pushed the fix-readme branch from 6020771 to 5e9c317 Compare January 13, 2025 08:16

Joel-hanson requested a review from gabortpubs January 13, 2025 08:22

dalelane reviewed Jan 13, 2025

View reviewed changes

Joel-hanson force-pushed the fix-readme branch from 5e9c317 to 4a621af Compare January 14, 2025 06:15

Joel-hanson requested a review from dalelane January 14, 2025 06:34

Joel-hanson force-pushed the fix-readme branch 3 times, most recently from 5d5c39b to 79d0e76 Compare January 14, 2025 10:01

dalelane approved these changes Jan 14, 2025

View reviewed changes

Update README.md

ed8da4b

Signed-off-by: Joel Hanson <[email protected]>

Joel-hanson force-pushed the fix-readme branch from 79d0e76 to ed8da4b Compare January 14, 2025 11:05

gabortpubs approved these changes Jan 14, 2025

View reviewed changes

dalelane merged commit 7eb0578 into ibm-messaging:main Jan 14, 2025
2 checks passed

Joel-hanson deleted the fix-readme branch January 14, 2025 11:45

	\| `root.element.name` \| `root` \| The name of the root element in the XML document being parsed. \|
	\| `root.element.name` \| `root` \| The name of the root element in the XML document that is being parsed. \|

	\| `xsd.schema.path` \| \| Location of a schema file to use to parse the XML string. \|
	\| `xsd.schema.path` \| \| The location of the schema file to use when parsing the XML string. \|

	\| `xml.doc.flat.enable` \| `false` \| Set to `true` if the XML strings contain a single value (e.g. `<root>the message</root>`) \|
	\| `xml.doc.flat.enable` \| `false` \| Set to `true` if the XML strings contain a single value (for example, `<root>the message</root>`) \|

	A Kafka Connect converter for converting between Kafka Connect’s internal data format and XML strings.
	A Kafka Connect converter for converting between structured objects (Kafka Connect’s internal data format & Java Maps and Lists) and XML strings.

fix: update readme to have proper wordings #9

fix: update readme to have proper wordings #9

Uh oh!

Conversation

Joel-hanson commented Jan 2, 2025

Description

Type of change

How Has This Been Tested?

Checklist

Uh oh!

gabortpubs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gabortpubs 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!

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!

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants