DROOLS-3727 - Adding kogito compileSourcesArtifact to business-central pom.xml #965

tiagodolphine · 2019-10-01T20:11:59Z

Allow initial kogito branch on kie-wb-common to be merged on master, adding some needed dependencies.

Related PR's
kiegroup/kie-wb-common#2923
kiegroup/droolsjbpm-build-bootstrap#1071

manstis

This change is also likely on every showcase kicking around.. but they can possibly be cleaned up as needed....

manstis · 2019-10-02T08:38:07Z

business-central-parent/business-central-webapp/pom.xml

@@ -2708,6 +2708,9 @@
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-forms-client</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-api</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-client</compileSourcesArtifact>
+	    <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-kogito-api</compileSourcesArtifact>


You couldn't fix the formatting could you!? Sorry.

why this needs to be added in BC? Is that API used in 7.x applications?
This is big -1 to include it in full and productized part of the buiild

done @manstis

jomarko

Code formatting.

jomarko · 2019-10-02T10:43:05Z

business-central-parent/business-central-webapp/pom.xml

@@ -2708,6 +2708,9 @@
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-forms-client</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-api</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-client</compileSourcesArtifact>
+	    <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-kogito-api</compileSourcesArtifact>


Please use the same indentation as lines above and below.

mareknovotny · 2019-10-02T12:05:31Z

business-central-parent/business-central-webapp/pom.xml

@@ -2708,6 +2708,9 @@
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-forms-client</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-api</compileSourcesArtifact>
            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-project-client</compileSourcesArtifact>
+	    <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-kogito-api</compileSourcesArtifact>
+            <compileSourcesArtifact>org.kie.workbench.stunner:kie-wb-common-stunner-kogito-client</compileSourcesArtifact>
+            <compileSourcesArtifact>org.kie.workbench:kie-wb-common-kogito-client</compileSourcesArtifact>


why this needs to be added in BC? Is that API used in 7.x applications? I would expect it is in kogito editors not in our 7.x stream as the name says.

The refactoring of Stunner to support re-use of the core editors in both Business Central and kogito (VSCode etc plugins) necessitated the introduction of new modules. The source code for the new modules needs to be declared to the GWT compiler in order for GWT compilation to succeed.

maybe you should use different naming then if that is general API for using by both code bases

I'm agree with @mareknovotny guys - I think kogito should be used for the impls rather than for APIs...

At least in Stunner we'd been always making the difference between: "standalone" and "project" stuff. For the implementation, most of the "project" related class names start with or contains some "Project" term, and the "standalone" ones are usually named using the term "Default" or "Standalone", but the API class names do not mention it...

So this way, in Stunner terms, the term "kogito" can be directly mapped to the "standalone" stuff, but not at API level, IMO at the implementation.

This is why I was proposing also to remove the "stunner-kogito-api" module and refactor that code into the core, it's fine then having the kogito, standalone, project or whatever impl on top, even as maven modules, but IMO that kogito API stuff should be integrated into the core, and not make the relationship on the API classes by using the term kogito

Just leaving my 2 cents here too guys, I don't say it's a blocker, it works anyway, but as far as we introduce something into master it gets visibility, so I would prefer to have this discussions now rather than later...

Thx!

@manstis I read @romartin 's comment and have feeling that in Stunner they use term standalone for client side marshalling version, while in Dmn we use standalone for server side marshalling version? Do I miss something?

@romartin Can you please clarify your terms?

DMN and scesim use -runtime for kogito standalone and -testing for kogito with wrapper (for development). We use the term -standalone for what was the project showcase.

hey @manstis @jomarko
In stunner there are two environments on top the core/commons libraries, so two showcaes too - the standalone and the project ones.

The term standalone or project has nothing to do with BPMN marshallers - those marshallers are either on bpm-backend (server) or bpmn-marshalling modules (client), and then, on of top those modules, we'll have in kogito 3 environments now:

standalone -> a webapp, with as less kie deps as possible, but not as kogito. It still uses the BPMN server sdie marshallers

project -> the webapp for the whole kie environment. It still uses the BPMN server sdie marshallers

bpmn-runtime-webapp -> the webapp for kogito's BPMN editor. It uses the client side marshallers

So as you can see, the terms standalone, project or kogito are (or should be) top level modules, on top of the core/commons ones for each domain (BPMN,DMN), and in case of BPMN, marshallers are not specifcly tight to any concrete environment, they're just maven deps thta can be added to achieve whatevet kind of marshallers you want.

Hope this clarifies a bit...

jomarko · 2019-10-15T05:29:27Z

@manstis please see my question #965 (comment)

romartin

Approving it assuming that we still need to refactor the following dependenices by remvoing from this pom - the idea is that KIE stuff should not depend on any kogito stuff:

org.kie.workbench.stunner:kie-wb-common-stunner-kogito-api
org.kie.workbench.stunner:kie-wb-common-stunner-kogito-client
org.kie.workbench:kie-wb-common-kogito-client

@tiagodolphine could you please report it?? thx!

jomarko · 2019-10-17T05:12:17Z

@manstis should we align this naming or it isn't in conflict from your point of view?

manstis · 2019-10-17T07:53:26Z

@jomarko The general agreement is to get DROOLS-3727 merged "as is" and perform a secondary clean-up/refactor/move of classes to remove the kogito dependencies from Business Central.

jomarko · 2019-10-17T07:58:48Z

@manstis sure, my quesiton was if I should create ticket to align project, standalone, and xxx-runtime-webapp naming for bpmn and dmn. Or is the current naming fine?

manstis · 2019-10-17T08:08:54Z

@jomarko

Personally I'm OK with them as they are:

BPMN terms
BPMN does not have a webapp for kogito testing.

kie-wb-common-stunner-showcase-standalone
kie-wb-common-stunner-showcase-project
kie-wb-common-stunner-bpmn-kogito-runtime

DMN terms
DMN does not have a webapp for project (we use drools-wb-webapp in these cases).

kie-wb-common-dmn-webapp-standalone
kie-wb-common-dmn-webapp-kogito-runtime
kie-wb-common-dmn-webapp-kogito-testing

mareknovotny · 2019-10-18T08:16:48Z

from FDB here we need some fix for GWT maven plugin to not complain about ERROR

21:19:38 [ERROR] Failed to execute goal org.codehaus.mojo:gwt-maven-plugin:2.8.2:compile (gwt-compile) on project business-central-webapp: Command failed with status 1 -> [Help 1]
21:19:38 org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:gwt-maven-plugin:2.8.2:compile (gwt-compile) on project business-central-webapp: Command failed with status 1

see more details https://rhba-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/KIE/job/master/job/pullrequest/job/kie-wb-common-downstream-pullrequests/446/console

mareknovotny · 2019-10-18T08:17:32Z

Jenkins retest this please

manstis · 2019-10-18T08:19:47Z

@hasys @mareknovotny I'm on the case.... it's likely to be some missing configuration in the pom and gwt.xml file....

…l pom.xml

manstis · 2019-10-18T09:24:50Z

Jenkins please retest this.

manstis · 2019-10-18T10:12:30Z

Jenkins please retest this...

[ERROR] 'dependencies.dependency.version' for org.kie.workbench:kie-wb-common-kogito-client:jar is missing. @ org.kie:business-central-webapp:[unknown-version], /home/jenkins/workspace/KIE/master/pullrequest/kie-wb-distributions-pullrequests/business-central-parent/business-central-webapp/pom.xml, line 2019, column 17
[ERROR] 'dependencies.dependency.version' for org.kie.workbench.stunner:kie-wb-common-stunner-kogito-api:jar is missing. @ org.kie:business-central-webapp:[unknown-version], /home/jenkins/workspace/KIE/master/pullrequest/kie-wb-distributions-pullrequests/business-central-parent/business-central-webapp/pom.xml, line 2025, column 17
[ERROR] 'dependencies.dependency.version' for org.kie.workbench.stunner:kie-wb-common-stunner-kogito-client:jar is missing. @ org.kie:business-central-webapp:[unknown-version], /home/jenkins/workspace/KIE/master/pullrequest/kie-wb-distributions-pullrequests/business-central-parent/business-central-webapp/pom.xml, line 2030, column 17

Dependency versions are declared:

https://github.com/kiegroup/droolsjbpm-build-bootstrap/pull/1071/files#diff-5861e0bc527e96c78922ce7e7c5e9c98R2346

https://github.com/kiegroup/droolsjbpm-build-bootstrap/pull/1071/files#diff-5861e0bc527e96c78922ce7e7c5e9c98R2689

The dependent PRs are all on the same branch name so this should be fine.....

mareknovotny · 2019-10-18T11:04:18Z

@manstis the PR build takes into account only PRs across repos if that is from the same author. so here it doesn't apply as this PR is from @tiagodolphine repo branch, but the other 2 PRs are directly from branches on the kiegroup repos.

manstis · 2019-10-18T11:04:51Z

Jenkins please execute full downstream build....

IDK why a normal build is not fetching the correct downstream branches :-(

GitHub repositories that will be cloned and built:
	git://github.com/errai/errai.git:master:master-pr-build
	git://github.com/kiegroup/lienzo-core.git:master:master-pr-build
	git://github.com/kiegroup/lienzo-tests.git:master:master-pr-build
	git://github.com/kiegroup/droolsjbpm-build-bootstrap.git:master:master-pr-build
	git://github.com/kiegroup/kie-soup.git:master:master-pr-build
	git://github.com/kiegroup/appformer.git:master:master-pr-build
	git://github.com/kiegroup/droolsjbpm-knowledge.git:master:master-pr-build
	git://github.com/kiegroup/drools.git:master:master-pr-build
	git://github.com/kiegroup/optaplanner.git:master:master-pr-build
	git://github.com/kiegroup/jbpm.git:master:master-pr-build
	git://github.com/kiegroup/kie-jpmml-integration.git:master:master-pr-build
	git://github.com/kiegroup/droolsjbpm-integration.git:master:master-pr-build
	git://github.com/kiegroup/openshift-drools-hacep.git:master:master-pr-build
	git://github.com/kiegroup/droolsjbpm-tools.git:master:master-pr-build
	git://github.com/kiegroup/kie-uberfire-extensions.git:master:master-pr-build
	git://github.com/kiegroup/kie-wb-playground.git:master:master-pr-build
	git://github.com/kiegroup/kie-wb-common.git:master:master-pr-build
	git://github.com/kiegroup/drools-wb.git:master:master-pr-build
	git://github.com/kiegroup/optaplanner-wb.git:master:master-pr-build
	git://github.com/kiegroup/jbpm-designer.git:master:master-pr-build
	git://github.com/kiegroup/jbpm-work-items.git:master:master-pr-build
	git://github.com/kiegroup/jbpm-wb.git:master:master-pr-build
	git://github.com/kiegroup/kie-docs.git:master:master-pr-build
	git://github.com/kiegroup/optaweb-employee-rostering.git:master:master-pr-build
	git://github.com/kiegroup/optaweb-vehicle-routing.git:master:master-pr-build

mareknovotny · 2019-10-18T11:07:09Z

@manstis look above to my comment

manstis · 2019-10-18T11:09:33Z

@tiagobento Given @mareknovotny comment you may need to merge this first into kiegroup/kie-wb-distributions/DROOLS-3727 and then do a new PR to merge that into kiegroup/kie-wb-distributions/master... there's nothing I can do (other than close all three PRs and resubmit them with me as the author and from kiegroup which is probably not worth the effort given you'll be around soon....)

manstis · 2019-10-18T11:10:22Z

Thank-you @mareknovotny for your insights :-)

tiagodolphine · 2019-10-18T12:22:51Z

@manstis ok, I'll open a PR to merge into kiegroup/kie-wb-distributions/DROOLS-3727.
Thanks

tiagodolphine · 2019-10-18T12:32:14Z

new PR opened #977
@manstis
@mareknovotny

manstis · 2019-10-18T12:33:18Z

@tiagobento Want to close this one?

Thanks for the new PR :-)

tiagodolphine · 2019-10-18T12:33:53Z

closing this PR since we opened #965

manstis · 2020-02-20T09:17:45Z

Hi @romartin @tiagodolphine I was having a chat with @pefernan about his PRs for "state control" that was leading to everything to need a new dependency on appformer-kogito-bridge (including Business Central etc) and it reminded me of this scenario.. where we know we need to remove "kogito" dependencies from Business Central etc.. Can anybody remember the JIRA we created to track it!?! I might try to fix it soon.

romartin · 2020-02-20T20:22:46Z

Hey @manstis

Good point!! I also remember that conversation... I'd been looking accross several JIRA tickets, starting from KOGITO-50, but I can't find any issue reported about "removing kogito dependencies from the BC...."

Sooo maybe we forgot to report it? If you can work on it, maybe you can just create the ticket as well... hehe better late than never :) The main goal, as far as I rememer, was avoiding having dependencies like those in the BC, right?

Otherwise please let me know and I'l lcreate the ticket, np, althoguh I won't be able to address it yet.

Thanks for pointing that out!!!

manstis · 2020-02-20T21:02:41Z

@romartin @tiagodolphine @pefernan Found it: https://issues.redhat.com/browse/KOGITO-459

Let's ensure the we don't add to the rot... ONLY kogito editors' modules should depend on kogito related modules (i.e. Business Central should not have ANY dependencies on kogito). The JIRA above should be remembered and added to sprints...... for some one, any one (probably me, the culprit of them in the first place!) to fix..

tiagodolphine requested review from manstis, hasys, romartin, jomarko and LuboTerifaj October 1, 2019 20:11

This was referenced Oct 1, 2019

DROOLS-3727 - Initial Kogito work branch into master kiegroup/kie-wb-common#2923

Merged

DROOLS-3727 - Initial Kogito work branch into master kiegroup/droolsjbpm-build-bootstrap#1071

Merged

manstis reviewed Oct 2, 2019

View reviewed changes

jomarko reviewed Oct 2, 2019

View reviewed changes

mareknovotny reviewed Oct 2, 2019

View reviewed changes

manstis approved these changes Oct 2, 2019

View reviewed changes

romartin approved these changes Oct 16, 2019

View reviewed changes

mareknovotny requested review from mareknovotny and manstis October 18, 2019 08:17

tiagodolphine and others added 3 commits October 18, 2019 10:23

DROOLS-3727 - Adding kogito compileSourcesArtifact to business-centra…

aa42a68

…l pom.xml

DROOLS-3727 fix identation

892a40d

DROOLS-3727: Add temporary kogito related module configuration

e8a17b3

manstis force-pushed the DROOLS-3727 branch from 3e81dbf to e8a17b3 Compare October 18, 2019 09:23

manstis approved these changes Oct 18, 2019

View reviewed changes

tiagodolphine closed this Oct 18, 2019

manstis mentioned this pull request Oct 21, 2019

DROOLS-3727 [upstream] - Adding kogito compileSourcesArtifact to business-central pom.xml #977

Merged

DROOLS-3727 - Adding kogito compileSourcesArtifact to business-central pom.xml #965

DROOLS-3727 - Adding kogito compileSourcesArtifact to business-central pom.xml #965

Conversation

tiagodolphine commented Oct 1, 2019

manstis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jomarko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mareknovotny Oct 2, 2019 • edited

Choose a reason for hiding this comment

romartin Oct 2, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jomarko commented Oct 15, 2019

romartin left a comment • edited

Choose a reason for hiding this comment

jomarko commented Oct 17, 2019

manstis commented Oct 17, 2019

jomarko commented Oct 17, 2019

manstis commented Oct 17, 2019

mareknovotny commented Oct 18, 2019

mareknovotny commented Oct 18, 2019

manstis commented Oct 18, 2019

manstis commented Oct 18, 2019

manstis commented Oct 18, 2019

mareknovotny commented Oct 18, 2019

manstis commented Oct 18, 2019

mareknovotny commented Oct 18, 2019

manstis commented Oct 18, 2019

manstis commented Oct 18, 2019

tiagodolphine commented Oct 18, 2019

tiagodolphine commented Oct 18, 2019

manstis commented Oct 18, 2019

tiagodolphine commented Oct 18, 2019 • edited

manstis commented Feb 20, 2020

romartin commented Feb 20, 2020

manstis commented Feb 20, 2020

mareknovotny Oct 2, 2019 •

edited

romartin Oct 2, 2019 •

edited

romartin left a comment •

edited

tiagodolphine commented Oct 18, 2019 •

edited