-
Notifications
You must be signed in to change notification settings - Fork 145
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
[Fix #2193] ExecuteAfter mutation to return Json #2194
Conversation
Other mutations are still returning String to keep backward compatibility, although we probably should change them too
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2194 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2194/1/display/redirect Test results:
Those are the test failures: org.kie.kogito.index.addon.api.KogitoAddonRuntimeClientImplTest.testExecuteAfterSuccessclass java.lang.Object cannot be cast to class org.kie.kogito.Model (java.lang.Object is in module java.base of loader 'bootstrap'; org.kie.kogito.Model is in unnamed module of loader 'app') |
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.
only a non blocker comment form my side.
} else { | ||
LOGGER.trace("Error {}", logMessage); | ||
LOGGER.info("Error {}", logMessage); |
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.
If we are logging an error, why not to use do LOGGER.error ?
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.
Looks good to me, good work @fjtirado !
* [Fix apache#2193] ExecuteAfter mutation to return Json Other mutations are still returning String to keep backward compatibility, although we probably should change them too * [Fix apache#2193] Walters comment
Fix #2193
Other mutations are still returning String to keep backward compatibility, although we probably should change them too