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

Fix issue 260 by adding a topic function. #264

Merged
merged 5 commits into from Mar 31, 2022
Merged

Fix issue 260 by adding a topic function. #264

merged 5 commits into from Mar 31, 2022

Conversation

gklijs
Copy link
Collaborator

@gklijs gklijs commented Mar 29, 2022 •
edited

Fix issue 260 by adding a topic function. Refactor how test are run, since build succeeded with failing integration tests, and coverage report was missing.
Fixes #260

@gklijs gklijs self-assigned this Mar 29, 2022
@gklijs gklijs force-pushed the feature260 branch 6 times, most recently from 0ee2824 to ad92584 Compare March 29, 2022 17:52
@smcvb smcvb added this to the Release 4.6.0 milestone Mar 30, 2022
@gklijs
Copy link
Collaborator Author

gklijs commented Mar 30, 2022

Tried to make the test setup more similar to framework, but still I have --- maven-failsafe-plugin:2.22.2:integration-test (run-integration-tests) @ axon-kafka --- and then actually no integration tests. Before the integration tests were running, but the build succeeded even with failures.

@gklijs gklijs requested a review from smcvb March 30, 2022 09:53
@gklijs
Copy link
Collaborator Author

gklijs commented Mar 30, 2022

Integration tests failing, should break the build now, and total coverage is reported again.

smcvb
smcvb reviewed Mar 30, 2022
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

A couple of pointers to cover. Nothing major to block over though.

.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
...framework/extensions/kafka/eventhandling/producer/DefaultProducerFactoryIntegrationTest.java Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
@gklijs
Fix issue 260 by adding a topic function. Refactor how test are run, …
51329d4 
…since build succeeded with failing integration tests, and coverage report was missing.
smcvb
smcvb requested changes Mar 31, 2022
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

A bit more components this time around. Let me know what you think.

.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
...a/src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/TopicResolver.java Show resolved Hide resolved
...a/src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/TopicResolver.java Show resolved Hide resolved
...a/src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/TopicResolver.java Show resolved Hide resolved
.../src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/KafkaPublisher.java Outdated Show resolved Hide resolved
...a/src/main/java/org/axonframework/extensions/kafka/eventhandling/producer/TopicResolver.java Outdated Show resolved Hide resolved
gklijs and others added 3 commits March 31, 2022 12:11
@gklijs @smcvb
Update kafka/src/main/java/org/axonframework/extensions/kafka/eventha…
b4ff65b 
…ndling/producer/KafkaPublisher.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update kafka/src/main/java/org/axonframework/extensions/kafka/eventha…
f2bbf29 
…ndling/producer/KafkaPublisher.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs
Fix pending issues
8d053b0
smcvb
smcvb approved these changes Mar 31, 2022
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Just two points remaining in there! I'll approve preemptively, as I trust that you'll solve those as intended.

@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.8% 93.8% Coverage
0.0% 0.0% Duplication

@gklijs gklijs merged commit 9af9bc7 into master Mar 31, 2022
@gklijs gklijs deleted the feature260 branch March 31, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smcvb smcvb smcvb approved these changes

Assignees

@gklijs gklijs

Labels
Priority 1: Must Status: Resolved Type: Feature
Projects
None yet
Milestone
Release 4.6.0
Development

Successfully merging this pull request may close these issues.

Allow filtering or sending different event to different topics in the publisher.
2 participants
@gklijs @smcvb