Copilot commented on code in PR #6704:
URL:
https://github.com/apache/incubator-kie-drools/pull/6704#discussion_r3248993039
##########
kie-api/src/main/java/org/kie/api/conf/OptionsConfiguration.java:
##########
@@ -28,13 +28,10 @@ public interface OptionsConfiguration<T extends Option, S
extends SingleValueOpt
void makeImmutable();
/**
- * Gets an option value
+ * Sets an option value
*
* @param option the option class for the option being requested
* @param <C extends T>
Review Comment:
The `@param` line for the type parameter uses `<C extends T>` with raw angle
brackets. In Javadoc HTML these will be interpreted as an HTML tag and the
type-parameter description will be lost from the generated documentation. Use
`@param <C>` (the standard Javadoc syntax for documenting a type parameter)
instead, or escape the brackets as `<C extends T>`.
The same issue applies to line 70 (`@param <C> the option type`) — that one
is correct — but please apply the same fix consistently to the `setOption`
Javadoc on line 34.
##########
kie-api/src/main/javadoc/overview.html:
##########
@@ -65,18 +64,17 @@
</p>
<p>
- Drools 8 introduced Rule Unit API as a recommended style which provides an
atomic module defining a set of rules and a set of strongly typed
+ Drools 8 introduced the Rule Unit API, which provides an atomic module
defining a set of rules and a set of strongly-typed
data sources:
+</p>
<pre>
MeasurementUnit measurementUnit = new MeasurementUnit();
- RuleUnitInstance<MeasurementUnit> instance =
RuleUnitProvider.get().createRuleUnitInstance(measurementUnit);
+ RuleUnitInstance<MeasurementUnit> instance =
RuleUnitProvider.get().createRuleUnitInstance(measurementUnit);
try {
- measurementUnit.getMeasurements().add(new Measurement("color",
"red"));
- List<Measurement> queryResult =
instance.executeQuery("FindColor").stream().map(tuple ->
(Measurement) tuple.get("$m")).collect(toList());
+ measurementUnit.getMeasurements().add(new Measurement("color", "red"));
+ List<Measurement> queryResult =
instance.executeQuery("FindColor").stream().map(tuple -> (Measurement)
tuple.get("$m")).collect(toList());
Review Comment:
These code samples in the Javadoc overview were previously HTML-escaped
(`<`/`>`/`"`) and have been replaced with raw `<` and `>`
characters. Because `overview.html` is rendered as HTML by the Javadoc tool,
the unescaped `<kmodule xmlns="..."/>` (line 52),
`RuleUnitInstance<MeasurementUnit>` (line 72) and `List<Measurement>` (line 75)
will be interpreted as (unknown) HTML tags and stripped from the rendered
output, so the type parameters and the kmodule snippet will disappear from the
generated Javadoc. These angle brackets need to remain HTML-escaped
(`<`/`>`).
##########
kie-api/src/main/java/org/kie/api/event/process/ProcessNodeTriggeredEvent.java:
##########
@@ -22,6 +22,11 @@
* An event when a node inside a process instance has been triggered.
*/
public interface ProcessNodeTriggeredEvent extends ProcessNodeEvent {
+ /**
+ * Indicates if this node was retriggered after an error.
+ * @return true if retriggered, false otherwise
+ * @since 10.1.0
+ */
default boolean isRetrigger() {
return false;
}
Review Comment:
This new method body uses tab characters for indentation, while the rest of
this file (and the kie-api module in general) uses 4-space indentation. Please
replace the tabs with spaces to match the surrounding code style.
##########
kie-api/pom.xml:
##########
@@ -79,7 +46,7 @@
<configuration>
<sourcepath>${project.basedir}/src/main/java</sourcepath>
<excludePackageNames>org.kie.util*</excludePackageNames>
- <source>${maven.compiler.release}</source>
+ <release>${maven.compiler.release}</release>
Review Comment:
The Javadoc plugin configuration was switched from `<source>` to
`<release>`. Note that with `<release>`, the Javadoc tool will compile against
and document only APIs available in that JDK release and reject `--source-path`
constructs that were tolerated by `<source>`. If `${maven.compiler.release}`
differs from the JDK used to run the build, this can change which APIs Javadoc
resolves and may produce warnings/errors that did not occur previously. Please
confirm this works under the project's CI JDK matrix.
##########
kie-api/src/main/java/org/kie/api/management/KieBaseConfigurationMonitorMBean.java:
##########
@@ -20,42 +20,82 @@
public interface KieBaseConfigurationMonitorMBean {
- @Deprecated
+ /**
+ * @return the alpha node hashing threshold
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public int getAlphaNodeHashingThreshold();
public String getAssertBehaviour();
- @Deprecated
+ /**
+ * @return the composite key depth
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public int getCompositeKeyDepth();
public String getEventProcessingMode();
- @Deprecated
+ /**
+ * @return the maximum number of threads
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public int getMaxThreads();
- @Deprecated
+ /**
+ * @return the sequential agenda setting
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public String getSequentialAgenda();
- @Deprecated
+ /**
+ * @return whether left beta memory is indexed
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isIndexLeftBetaMemory();
- @Deprecated
+ /**
+ * @return whether right beta memory is indexed
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isIndexRightBetaMemory();
- @Deprecated
+ /**
+ * @return whether TMS is maintained
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isMaintainTms();
public boolean isMBeansEnabled();
public boolean isRemoveIdentities();
- @Deprecated
+ /**
+ * @return whether sequential mode is enabled
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isSequential();
- @Deprecated
+ /**
+ * @return whether alpha nodes are shared
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isShareAlphaNodes();
- @Deprecated
+ /**
+ * @return whether beta nodes are shared
+ * @deprecated since 8.35.0, this configuration option is no longer used.
Will be removed in a future version.
+ */
+ @Deprecated(since = "8.35.0", forRemoval = true)
public boolean isShareBetaNodes();
Review Comment:
The deprecation reasons added throughout `KieBaseConfigurationMonitorMBean`
all state "this configuration option is no longer used" and use a `since =
"8.35.0"` value, but no source/citation in the PR establishes when each
individual option actually became unused or that 8.35.0 is the correct
deprecation version for all of them — these methods have been `@Deprecated`
(without `since`) for a long time. Please verify the `since` value for each
method against git history rather than applying a single uniform version,
otherwise the published API documentation will report inaccurate deprecation
history.
##########
kie-api/src/main/java/org/kie/api/runtime/RuntimeSession.java:
##########
@@ -22,6 +22,12 @@
import java.util.Map;
+/**
+ * Common interface for runtime session operations shared by stateful and
stateless sessions.
+ * @since 10.1.0
+ * @apiNote Use this interface when writing code that works with both
KieRuntime
+ * and StatelessKieSession to avoid coupling to specific session
types.
+ */
Review Comment:
The `@since 10.1.0` tag is being added to the `RuntimeSession` interface as
if it were new in 10.1.0, but this interface already exists on the main branch
and was not introduced by this PR. Adding an `@since` tag with a version that
does not match the actual introduction version is misleading documentation.
Either remove the `@since` tag or set it to the version in which
`RuntimeSession` was actually first released.
The same concern applies to the other `@since 10.1.0` / `@since 10.2.0` tags
added in this PR to pre-existing types and methods (e.g., `KieRuntime`,
`StatelessKieSession`, `NodeInstanceContainer.getSerializableNodeInstances`,
`KieRuntimeBuilder.newStatelessKieSession`,
`KieCommands.newBatchExecution(Command...)`,
`ProcessNodeTriggeredEvent.isRetrigger`, `ProcessRetriggeredEvent`,
`ProcessStateChangeEvent`, `ProcessNodeStateChangeEvent`,
`ProcessEventListener.onProcessStateChanged`/`onNodeStateChanged`). Each
`@since` should reflect the version in which the API element was actually
introduced; please verify these against git history before merging.
##########
kie-api/src/main/java/org/kie/api/persistence/jpa/KieStoreServices.java:
##########
@@ -32,8 +32,16 @@ KieSession newKieSession(KieBase kbase,
/**
* Deprecated use {@link #loadKieSession(Long, KieBase,
KieSessionConfiguration, Environment)} instead
+ *
+ * @param id the session id
+ * @param kbase the KieBase
+ * @param configuration the session configuration
+ * @param environment the environment
+ * @return the loaded KieSession
+ * @deprecated since 6.3.0, use {@link #loadKieSession(Long, KieBase,
KieSessionConfiguration, Environment)} instead.
+ * This method uses int which can overflow for long-running
sessions. Will be removed in a future version.
Review Comment:
The `@deprecated` Javadoc claims "This method uses int which can overflow
for long-running sessions." This justification is incorrect for
`loadKieSession(int id, ...)` — the parameter being `int` does not overflow on
its own; sessions accumulate identifiers. More importantly, the same
explanation was copied from `KieSession.getId()` (where the concern was about
the return value, not a parameter). Please update the deprecation reason to
accurately describe why this overload is deprecated (the replacement takes a
`Long` id with wider range).
##########
build-parent/pom.xml:
##########
@@ -213,7 +213,15 @@
<!-- These are added as part of the migration from JBoss to Apache parent
pom.xml. They may be extracted to a KIE parent bom. -->
<version.maven-checkstyle>3.3.0</version.maven-checkstyle>
<version.build.helper.maven.plugin>3.4.0</version.build.helper.maven.plugin>
-
<version.maven-javadoc-plugin.override>3.6.2</version.maven-javadoc-plugin.override>
+ <version.maven-javadoc-plugin>3.12.0</version.maven-javadoc-plugin>
+
+ <!-- Javadoc branding properties - can be overridden via -D flags or CI
environment -->
+ <javadoc.product.name>${project.name}</javadoc.product.name>
+ <javadoc.product.version>${project.version}</javadoc.product.version>
+ <javadoc.doctitle>${project.name} ${project.version} API</javadoc.doctitle>
+ <javadoc.windowtitle>${project.name} ${project.version}
API</javadoc.windowtitle>
+ <javadoc.footer><![CDATA[Copyright ©
${project.inceptionYear}–2026 Apache Software Foundation. All rights
reserved.]]></javadoc.footer>
+ <javadoc.bottom><![CDATA[Copyright ©
${project.inceptionYear}–2026 Apache Software Foundation. All rights
reserved.]]></javadoc.bottom>
Review Comment:
The copyright footer/bottom hard-codes the end year as `2026`, which means
the published Javadoc will become stale every January and require a manual
edit. Consider using a build-time computed year (e.g. via
`build-helper-maven-plugin`'s `timestamp-property`/`maven.build.timestamp`) so
the year automatically tracks the build date.
##########
kie-api/README.md:
##########
@@ -0,0 +1,156 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ -->
+
+# KIE API
+
+The Apache KIE Public API module provides the core interfaces and classes for
the Drools, DMN and jBPM rule engines. This API aims to maintain backwards
compatibility within minor and patch releases, following semantic versioning
principles.
+
+## API Change Management
+
+Apache KIE follows [Semantic Versioning](https://semver.org/) since release
10.3.0:
+- **Major version** (X.0.0): Breaking changes allowed
+- **Minor version** (0.X.0): New features, backwards compatible
+- **Patch version** (0.0.X): Bug fixes only, backwards compatible
+
+### API Deprecation and Removal
+
+- APIs planned for removal must be **deprecated first** and can only be
removed in a **major release**
+- Deprecation must include:
+ - `@Deprecated` annotation with `since` and `forRemoval` attributes
+ - Javadoc explaining why it's deprecated and the recommended alternative
+ - Exception: When an entire feature is removed, no alternative may exist
+
+### API Changes
+
+- **Changing an API**: Deprecate the existing API and introduce the new one.
Redirect users via javadoc
+- **New APIs**: Must include `@since` javadoc tag indicating the version when
introduced
+- **Documentation**: All public APIs must have comprehensive javadoc
+
+## API Compatibility Checking
+
+This module uses [Revapi](https://revapi.org/) as the primary tool for API
compatibility checking and enforcement.
+
+Additionally, [JApiCmp](https://github.com/siom79/japicmp) is used to generate
supplementary compatibility reports for analysis purposes.
+
+### Revapi
+
+#### Purpose
+Revapi automatically detects breaking changes in the public API by comparing
the current code against a previous release, helping maintain backwards
compatibility.
+
+#### Usage
+
+The `revapi.latestVersion` property defines which version to compare against.
The current version is defined in `build-parent/pom.xml`.
+
+```bash
+# Check API compatibility (uses revapi.latestVersion from global settings)
+mvn clean verify
+
+# Or specify version explicitly (check build-parent/pom.xml for current
version)
+mvn clean verify -Drevapi.latestVersion=<version>
+
+# Generate detailed HTML report
+mvn clean package -Drevapi.latestVersion=<version>
+# Report: target/site/revapi-report.html
+```
+
+#### Handling API Changes
+
+If Revapi detects API changes and you need to make **intentional** breaking or
non-breaking changes:
+
+1. Review the reported changes carefully
+2. Update `src/build/revapi-config.json` to approve the changes
+3. Add each change to the `ignore` array with:
+ - Description of the change
+ - Justification explaining why it's necessary
+
+**Example:**
+```json
+[
+ {
+ "extension": "revapi.differences",
+ "configuration": {
+ "ignore": true,
+ "differences": [
+ {
+ "code": "java.method.addedToInterface",
+ "new": "method void org.kie.api.KieSession::newMethod()",
+ "justification": "kie-issues#xyz: Added new method for feature X"
+ }
+ ]
+ }
+ }
+]
+```
+
+#### Configuration
+- **Config file**: `src/build/revapi-config.json`
+- **Excludes**: Internal packages (`org.kie.internal.*`,
`org.kie.api.internal.*`)
+- **Tracks**: Deprecations and API changes
+
+For detailed documentation, see:
+- [REVAPI-USAGE.md](REVAPI-USAGE.md) - Quick start guide
+- [REVAPI-CONFIGURATION.md](REVAPI-CONFIGURATION.md) - Configuration options
+- [API-STABILITY-GUIDE.md](API-STABILITY-GUIDE.md) - API stability guidelines
Review Comment:
The README references three companion documents — `REVAPI-USAGE.md`,
`REVAPI-CONFIGURATION.md`, and `API-STABILITY-GUIDE.md` — but none of these
files are added in this PR. These broken links should either be removed or the
corresponding files should be added to the module.
--
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]
