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

KOGITO-5533: implement evaluation of rules via CloudEvents #1610

Merged
merged 28 commits into from
Sep 29, 2021

Conversation

kostola
Copy link
Contributor

@kostola kostola commented Sep 16, 2021

@danielezonca
Copy link
Contributor

Let's wait this other PR to be merged https://github.com/kiegroup/kogito-runtimes/pull/1554

After that PR also this addon will become a quarkus extension so it will require some work

@ricardozanini FYI

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

The (build) Examples check has failed. Please check the logs.

@kostola
Copy link
Contributor Author

kostola commented Sep 17, 2021

jenkins retest this please

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 20, 2021

The (build) Apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 22, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 23, 2021

The (build) Examples check has failed. Please check the logs.

@ricardozanini
Copy link
Member

Jenkins rerun tests

Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Nicely done! Can you please consider adding a README.md in the common/event/rules folder? Just add a brief description of this add-on, the examples related to it and a link to the full documentation. I understand you will also introduce a new doc section, right? Many thanks!

@@ -36,21 +36,27 @@
public static final String KOGITO_DMN_EVALUATE_DECISION = "kogitodmnevaldecision";
public static final String KOGITO_DMN_FULL_RESULT = "kogitodmnfullresult";
public static final String KOGITO_DMN_FILTERED_CTX = "kogitodmnfilteredctx";
public static final String KOGITO_RULE_UNIT_ID = "kogitoruleunitid";
public static final String KOGITO_RULE_UNIT_QUERY = "kogitoruleunitquery";
Copy link
Member

Choose a reason for hiding this comment

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

Rules and decisions share the same extension points? Maybe we should consider decoupling those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decoupled them as suggested in your other comment.
@ricardozanini @danielezonca shall I rename KogitoExtension to KogitoDecisionsExtension in this PR or is it better to create a dedicated task to fix naming?

Copy link
Member

Choose a reason for hiding this comment

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

Many thanks! It's better to handle it in another PR. :)

}

public static String toKebabCase(String inputString) {
return inputString.replaceAll("(.)(\\p{Upper})", "$1-$2").toLowerCase();
Copy link
Member

Choose a reason for hiding this comment

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

We already have this code in the add-on. Can we have a shared library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ricardozanini is it ok for you if I refactor this in a separated PR?

@kostola kostola requested a review from jiripetrlik September 29, 2021 10:04
@kie-ci
Copy link
Contributor

kie-ci commented Sep 29, 2021

The (build) Examples check has failed. Please check the logs.

@ricardozanini
Copy link
Member

Many thanks, @kostola! Please don't forget to open the follow-up JIRAs.

@kie-ci
Copy link
Contributor

kie-ci commented Sep 29, 2021

The (build) Examples check has failed. Please check the logs.

@kostola
Copy link
Contributor Author

kostola commented Sep 29, 2021

Many thanks, @kostola! Please don't forget to open the follow-up JIRAs.

@ricardozanini I created the task for the remaining minor refactoring: https://issues.redhat.com/browse/KOGITO-6020

@kie-ci
Copy link
Contributor

kie-ci commented Sep 29, 2021

The (build) Apps check has failed. Please check the logs.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

25.4% 25.4% Coverage
0.0% 0.0% Duplication

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.

5 participants