-
Notifications
You must be signed in to change notification settings - Fork 9
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
[DROOLS-7578] Evaluate Impact analysis for ansible integration rules #94
[DROOLS-7578] Evaluate Impact analysis for ansible integration rules #94
Conversation
Next task: Export the graph as JSON (implement in drools-impact-analysis side) so that Eder's UI can consume. |
Run CI after the 8.45.0-SNAPSHOT update is deployed. |
- Quick prototype. Works for a basic example : ParserTest - Replace jboss repo with apache repo for snapshot
1c98fd0
to
42f1fa7
Compare
- project: kiegroup/drools | ||
- project: apache/incubator-kie-drools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now changed from kiegroup/drools
to apache/incubator-kie-drools
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lampajr , this is a little irregular case. drools-ansible-rulebook-integration
is developed in kiegroup
, but it depends on the latest SNAPSHOT of apache/incubator-kie-drools
. Do you think these yaml changes are valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tkobayas, If I am not missing anything the change is good to adapt the PR checks as the only change is the drools
org here (so I am not expecting anything else).
Let me tag @rgdoliveira such that he can also review it as he is much more involved.
PS: I think the same change should be backported to 1.0.x
branch if still in active development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for reviewing, @lampajr !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkobayas I think this change is ok. But keep in mind that the idea is that we will have a kiegroup/drools main branch in sync with apache/incubator-kie-drools at some point (that will be our midstream). So maybe later we can move back to kiegroup/drools when that branch is in place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rgdoliveira I wasn't aware that we wanted to restore kiegroup/drools and keep it in sync with the apache project (and tbh I don't see why we should), but anyway are you ok with this change for now? If so can you please approve it so I could merge? Thank you.
lhs.addPattern(pattern); | ||
} | ||
|
||
private static void parseConditions(Condition condition, Pattern pattern) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, LhsParser
and RhsParser
can handle simple rules. After we will get more realistic and complex rules, we can improve further.
<id>jboss-public-repository-group</id> | ||
<name>JBoss Public Repository Group</name> | ||
<url>https://repository.jboss.org/nexus/content/groups/public/</url> | ||
<id>apache-public-repository-group</id> | ||
<name>Apache Public Repository Group</name> | ||
<url>https://repository.apache.org/content/groups/public/</url> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use apache repo rather than jboss repo even for SNAPSHOT
@mariofusco Please review, thanks! |
Note that this visualization depends on apache/incubator-kie-drools#5594 , so this doesn't run with drools |
This seems to be expected checkout.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI changes look good to me
Requires apache/incubator-kie-drools#5594 to compile and run.
drools
anddrools-ansible-rulebook-integration
have different upstream repos ,so cross-repository build wouldn't work, hence this CI should fail until apache/incubator-kie-drools#5594 is merged.