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

Feature/axon kotlin test #73

Merged
merged 6 commits into from
Nov 26, 2020

Conversation

zambrovski
Copy link
Collaborator

@zambrovski zambrovski commented Oct 19, 2020

Will fix #67

Copy link
Member

@sandjelkovic sandjelkovic left a comment

Choose a reason for hiding this comment

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

The package name should be changed to indicate that these are test utilities and differentiate from the regular extension methods. Appending .test to the package should be enough, as there's no need to split aggregate and saga utilities for now.
We can always segregate it in the future if there's the need for it.

Otherwise, looks good to me 👍

@sonarcloud
Copy link

sonarcloud bot commented Nov 3, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

Looks good to me 👍

@smcvb smcvb merged commit 62975cf into AxonFramework:master Nov 26, 2020
Copy link

@Arnaud-J Arnaud-J left a comment

Choose a reason for hiding this comment

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

Hi!

After trying this code locally for my project, I had trouble using the defined functions. I am still very new to Kotlin, therefore I might be missing something.
Nonetheless, here are some comments on how I would refactor this code. Please tell me if I am on the wrong path.

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.

Improve AggregateTextFixture
4 participants