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.

Sign up for GitHub

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

Jump to bottom

GH-3617 : Value parameters in ReactiveKafkaProducerTemplate should be nullable #3651

Merged
merged 3 commits into from
Dec 7, 2024
Merged

GH-3617 : Value parameters in ReactiveKafkaProducerTemplate should be nullable #3651

merged 3 commits into from
Dec 7, 2024

Conversation

jukekxm
Copy link
Contributor

@jukekxm jukekxm commented Nov 26, 2024

@pivotal-cla
Copy link

@kimjuke Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@kimjuke Thank you for signing the Contributor License Agreement!

@jukekxm jukekxm mentioned this pull request Nov 26, 2024
Closed
sobychacko
sobychacko reviewed Nov 27, 2024
View reviewed changes
...fka/src/main/java/org/springframework/kafka/core/reactive/ReactiveKafkaProducerTemplate.java Outdated
@@ -33,6 +33,7 @@
import reactor.kafka.sender.SenderRecord;
import reactor.kafka.sender.SenderResult;
import reactor.kafka.sender.TransactionManager;
import reactor.util.annotation.Nullable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using the Nullable annotation from the reactor project? Since we are in a Spring library, using Spring's org.springframework.lang.Nullable is okay. Could you change it?

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 your review!
I modified the package as you mentioned.

sobychacko
sobychacko requested changes Nov 27, 2024
View reviewed changes
Copy link
Contributor

@sobychacko sobychacko left a comment

Choose a reason for hiding this comment

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

Could you add your name to the author list?

@jukekxm
Copy link
Contributor Author

jukekxm commented Nov 28, 2024 •
edited
Loading

Could you add your name to the author list?

@sobychacko I added my name, Thank you.

@jukekxm jukekxm requested a review from sobychacko December 1, 2024 05:19
@sobychacko
Copy link
Contributor

PR is failing due to checkstyle errors. Can you fix those?

Error: eckstyle] [ERROR] /home/runner/work/spring-kafka/spring-kafka/spring-kafka/src/main/java/org/springframework/kafka/core/reactive/ReactiveKafkaProducerTemplate.java:29:1: 'org.springframework.lang.Nullable' should be separated from previous imports. [ImportOrder]

Error: eckstyle] [ERROR] /home/runner/work/spring-kafka/spring-kafka/spring-kafka/src/main/java/org/springframework/kafka/core/reactive/ReactiveKafkaProducerTemplate.java:30:1: Wrong order for 'reactor.core.publisher.Flux' import. [ImportOrder]

@jukekxm
Copy link
Contributor Author

jukekxm commented Dec 4, 2024

PR is failing due to checkstyle errors. Can you fix those?

Error: eckstyle] [ERROR] /home/runner/work/spring-kafka/spring-kafka/spring-kafka/src/main/java/org/springframework/kafka/core/reactive/ReactiveKafkaProducerTemplate.java:29:1: 'org.springframework.lang.Nullable' should be separated from previous imports. [ImportOrder]

Error: eckstyle] [ERROR] /home/runner/work/spring-kafka/spring-kafka/spring-kafka/src/main/java/org/springframework/kafka/core/reactive/ReactiveKafkaProducerTemplate.java:30:1: Wrong order for 'reactor.core.publisher.Flux' import. [ImportOrder]

I fixed them. Thank You :)

@sobychacko sobychacko merged commit e27a344 into spring-projects:main Dec 7, 2024
3 checks passed
spring-builds pushed a commit that referenced this pull request Dec 7, 2024
@jukekxm @spring-builds
GH-3617 : Value parameters in ReactiveKafkaProducerTemplate should be…
88026ed 
… nullable (#3651)

Fixes: #3617

* add @nullable parameter to send method
* change package for @nullable
* fix import order checkstyle

(cherry picked from commit e27a344)
@sobychacko
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobychacko sobychacko sobychacko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Value parameters in ReactiveKafkaProducerTemplate should be nullable
3 participants
@jukekxm @pivotal-cla @sobychacko