Skip to content

STORM-3879 Add Kafka README file to distribution#8429

Closed
MNkulkarni06 wants to merge 6 commits intoapache:masterfrom
MNkulkarni06:STORM-3879-add-kafka-readme
Closed

STORM-3879 Add Kafka README file to distribution#8429
MNkulkarni06 wants to merge 6 commits intoapache:masterfrom
MNkulkarni06:STORM-3879-add-kafka-readme

Conversation

@MNkulkarni06
Copy link
Contributor

What is the purpose of the change

This change ensures that the README files for the Kafka modules are included in the Storm distribution tar.
Previously, the following files were present in the repository but were not guaranteed to be included in the release package:

external/storm-kafka-migration/README.md
external/storm-kafka-monitor/README.md

Including these files in the distribution ensures that users have access to the module documentation when downloading the Storm release.

How was the change tested

The change was verified by reviewing the assembly configuration and confirming that the README files are included in the distribution packaging configuration.
Since this change only modifies the assembly descriptor and does not affect runtime code, no additional functional tests were required.

Copy link
Contributor

@rzo1 rzo1 left a comment

Choose a reason for hiding this comment

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

Can you drop the SSL part from this PR please?

@MNkulkarni06 MNkulkarni06 force-pushed the STORM-3879-add-kafka-readme branch from c3cd260 to 8981b71 Compare March 17, 2026 12:31
@MNkulkarni06
Copy link
Contributor Author

Thanks for the feedback. I have removed the incorrect SSL setup section and retained only the valid existing configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

These changes can be removed. They do not belong to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review.

I have removed the incorrect SSL-related changes. The PR now only contains the intended updates.

Please let me know if any further changes are needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but if you look into the actual changeset of this PR, you will notice, that the last push removed the README changes and let the TLS/SSL stuff inside the PR: 7ac7684

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification.

I have removed all SSL/TLS-related changes from this PR and kept only the intended updates.

Please let me know if anything else needs to be adjusted.

@rzo1
Copy link
Contributor

rzo1 commented Mar 20, 2026

This PR is currently empty. I am going to close it. Feel free to open a new one with the targeted changes regarding the README file.

@rzo1 rzo1 closed this Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants