Skip to content
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

Polish DestinationTopicPropertiesFactory #3004

Conversation

Wzy19930507
Copy link
Contributor

@Wzy19930507 Wzy19930507 commented Jan 26, 2024

polish DestinationTopicPropertiesFactory and DestinationTopicPropertiesFactoryTests

@sobychacko
Copy link
Contributor

sobychacko commented Jan 26, 2024

@Wzy19930507 Changes look good. Could you add a small description behind the motivation for this PR as a commit message or here? That way, it is easier to follow through, especially later on. Was there anything particularly wrong, or is it mostly cleanup?

@Wzy19930507
Copy link
Contributor Author

Wzy19930507 commented Jan 26, 2024

@sobychacko Hi, thanks for your review and good advice. It mostly cleanup.
Motivation is because has experienced many times of iteration, DestinationTopicPropertiesFactory is difficult for me to read

@Wzy19930507 Wzy19930507 force-pushed the refactor_destination_topic_properties_factory branch from 12be4c5 to 1a9d63d Compare January 26, 2024 20:22
@Wzy19930507 Wzy19930507 changed the title Refactor DestinationTopicPropertiesFactory Polish DestinationTopicPropertiesFactory Jan 26, 2024
@sobychacko
Copy link
Contributor

Can you update the copyright years on the classes changed?

@sobychacko sobychacko added this to the 3.2.0-M1 milestone Jan 30, 2024
@sobychacko sobychacko merged commit 69f201d into spring-projects:main Jan 30, 2024
3 checks passed
@sobychacko
Copy link
Contributor

@Wzy19930507 Thanks for the PR. It is now merged upstream.

@Wzy19930507 Wzy19930507 deleted the refactor_destination_topic_properties_factory branch January 30, 2024 15:31
@Wzy19930507
Copy link
Contributor Author

Thanks for your review!

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

Successfully merging this pull request may close these issues.

2 participants