gitgabrio commented on code in PR #5860:
URL:
https://github.com/apache/incubator-kie-drools/pull/5860#discussion_r1575793179
##########
drools-quarkus-extension/drools-quarkus-integration-test/src/main/resources/application.properties:
##########
@@ -17,4 +17,33 @@
# under the License.
#
-drools.prototypes=allowed
\ No newline at end of file
+drools.prototypes=allowed
Review Comment:
@mariofusco
Would it be possible to have to integration tests, one that verify
kmodule.xml, and the other that verifies application.properties ?
Beside - what would happen if both application.properties AND kmodule.xml
are defined ?
##########
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/project/RuleCodegen.java:
##########
@@ -110,12 +110,19 @@ public RuleCodegen withHotReloadMode() {
hotReloadMode = true;
return this;
}
-
+
+ public RuleCodegen withKieBaseModels(Map<String, KieBaseModel>
kieBaseModels) {
+ if (!kieBaseModels.isEmpty()) {
+ this.kieBaseModels = kieBaseModels;
+ }
+ return this;
+ }
+
public Collection<PackageModel> getPackageModels() {
return packageModels;
}
public Collection<KieBaseModel> getKmoduleKieBaseModels() {
- return kmoduleKieBaseModels;
+ return kieBaseModels == null ? null : kieBaseModels.values();
Review Comment:
Would it be possible to avoid returning `null`, that it is extremely
`NullPointerException` prone, and instead returns an empty collection ?
##########
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/project/KieSessionModelBuilder.java:
##########
@@ -45,8 +45,12 @@ public class KieSessionModelBuilder {
private DroolsModelBuildContext context;
public KieSessionModelBuilder(DroolsModelBuildContext context,
Collection<CodegenPackageSources> packageSources) {
+ this(context, packageSources, null);
Review Comment:
hi @mariofusco
Could you pls refactor those two constructors ?
1. instead of passing `null` here, invokes `
KieModuleModelWrapper.fromResourcePaths(context.getAppPaths().getPaths()).kieBaseModels();`
2. in the next constructor, instead of putting an `if` logic, enforce the
needs of `Map<String, KieBaseModel> kieBaseModels`
That way
1. the code is clearer
2. the meaning of the two constructors is better defined
wdyt ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
