Skip to content

Adv/code review #1229

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

Adv/code review #1229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b342c1c
Add code-review scaffolding and suppressions for Chronicle-Wire
peter-lawrey Oct 30, 2025
2588480
Move Checkstyle config under src/main/config
peter-lawrey Oct 30, 2025
25723bf
Add follow-up tasks for Chronicle values
peter-lawrey Oct 30, 2025
62e8197
Merge branch 'develop' into adv/code-review
peter-lawrey Oct 30, 2025
c1cbe1b
Apply pending changes across Chronicle-Wire
peter-lawrey Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions EventsByMethod.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,8 @@ Some advantages of Trivially Copyable Objects

Events received by a component can be handled in two main ways:

1. *Raw Message Forwarding* - the raw `Bytes` or `DocumentContext` can be forwarded to another queue or network channel without deserialising the message fully.
This is efficient for routers or filters that only inspect metadata.
2. *Full Deserialisation and Re-invocation* - the event is deserialised and the corresponding method invoked locally; as part of the processing logic the same (or another) method can then be invoked downstream via a `MethodWriter`.
. *Raw Message Forwarding* - the raw `Bytes` or `DocumentContext` can be forwarded to another queue or network channel without deserialising the message fully. This is efficient for routers or filters that only inspect metadata.
. *Full Deserialisation and Re-invocation* - the event is deserialised and the corresponding method invoked locally; as part of the processing logic the same (or another) method can then be invoked downstream via a `MethodWriter`.

== Modelling simple asynchronous method calls

Expand Down
13 changes: 7 additions & 6 deletions README.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,8 @@ prints:
Create configuration files cfg1.yaml and cfg2.yaml.
See how the classes' names appear differently in yaml files for classes with alias name and without alias name.
For Data1 object alias name is used in the yaml file (cfg1.yaml).
Data2 object should be loaded from a yaml file with the complete name of class (including package name) otherwise you will receive an Exception.
Data2 object should be loaded from a yaml file
with the complete name of class (including package name) otherwise you will receive an Exception.

.cfg1.yaml
[source,yaml]
Expand Down Expand Up @@ -1042,8 +1043,7 @@ In order to `reset` a marshallable object, the process is as follows:

. create a new instance of the object to be reset (this is done just once per object type, then cached)
. reset all fields, so that
.. when the field value implements `Resettable` interface, and the default value of the field is not `null`, and the current field value is of exactly the same class as the default value, `value.reset()` will be called.
This behavior is introduced in Chronicle Wire `2.25`.
.. when the field value implements `Resettable` interface, and the default value of the field is not `null`, and the current field value is of exactly the same class as the default value, `value.reset()` will be called. This behavior is introduced in Chronicle Wire `2.25`.
.. when the field type is `Collection` or `Map`, and the default value is empty collection, it will be cleared.
.. there is specific built-in copy-by-value handling for some types such as `Bytes`
.. all other fields will be copied by reference from the new instance to the existing instance
Expand Down Expand Up @@ -1143,7 +1143,8 @@ Other types are supported; for example, 32-bit integer values, and an array of 6
== Pass-by-name or Dynamic Enums

Chronicle Wire supports passing objects reference by the `name()` of the object referenced.
This is supported trivially with `enum` which define a `name()` for you. e.g.
This is supported trivially with `enum` which define a `name()` for you.
e.g.

.Passing a reference to an enum using it's name
[source,java]
Expand Down Expand Up @@ -1536,7 +1537,7 @@ You may wish to intercept handling a call in the method reader in order to execu
==== Intercepting by passing control over the original method call

`MethodReader` provides a flexible feature for supporting all the above use cases -- the option to specify `MethodReaderInterceptorReturns`.
If set, it will be triggered *instead* of the original call.
If set, it will be triggered _instead_ of the original call.
It's possible to either skip the original method or to call it via passed `Invocation` instance:

[source,Java]
Expand Down Expand Up @@ -1704,7 +1705,7 @@ The documentation looks well-thought-out, and it is worth emulating.
| Unset fields take space on wire? | optional | optional | no | yes | yes | no
| Pointers take space on wire? | no | no | no | yes | no | yes
| Pass-by-name (Dynamic Enums) | yes | no | no | no | no | no
| C` | yes | planned | yes | yes (C`11)* | yes | yes
| C{pp} | yes | planned | yes | yes (C`11)* | yes | yes
| Java | Java 8 | Java 8 | yes | yes* | yes | yes
| C# | yes | yes | yes | yes* | yes | yes*
| Go | no | no | yes | yes | no | yes*
Expand Down
9 changes: 9 additions & 0 deletions TODO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Chronicle-Wire Follow-up Tasks

Chronicle Software

## Code-Review Profile

- [ ] Chronicle-Wire: SpotBugs suppressions currently mask ≈285 findings (AA assertions, NP hot paths, constructor hygiene); triage and replace with real fixes where feasible.
- [ ] Chronicle-Wire: Current coverage 69.8% lines / 64.1% branches (JaCoCo thresholds set to 0.0/0.0 for code-review); raise gates once new tests land.
- [ ] Chronicle-Wire: Monitor PMD-4578 (nested generic parsing crash); remove `pom.xml` PMD excludes for TextWire/YamlWireOut once upstream ships a fix.
172 changes: 170 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
~ limitations under the License.
-->

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>net.openhft</groupId>
<artifactId>java-parent-pom</artifactId>
<version>1.27ea1</version>
<relativePath />
<relativePath/>
</parent>

<artifactId>chronicle-wire</artifactId>
Expand All @@ -36,6 +37,15 @@
<!-- Settings for SonarCloud analysis -->
<sonar.organization>openhft</sonar.organization>
<sonar.host.url>https://sonarcloud.io</sonar.host.url>
<checkstyle.version>3.6.0</checkstyle.version>
<puppycrawl.version>8.45.1</puppycrawl.version>
<spotbugs.version>4.8.6.6</spotbugs.version>
<findsecbugs.version>1.14.0</findsecbugs.version>
<maven-pmd-plugin.version>3.28.0</maven-pmd-plugin.version>
<jacoco-maven-plugin.version>0.8.14</jacoco-maven-plugin.version>
<chronicle-quality-rules.version>1.23ea6</chronicle-quality-rules.version>
<jacoco.line.coverage>0.80</jacoco.line.coverage>
<jacoco.branch.coverage>0.70</jacoco.branch.coverage>
</properties>

<!-- Use BOMs to keep dependency versions consistent across modules -->
Expand Down Expand Up @@ -436,6 +446,164 @@
</scm>

<profiles>
<!-- Profile to enforce code quality gates prior to review -->
<profile>
<id>code-review</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>
<jacoco.line.coverage>0.0</jacoco.line.coverage>
<jacoco.branch.coverage>0.0</jacoco.branch.coverage>
</properties>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>${checkstyle.version}</version>
<dependencies>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>${puppycrawl.version}</version>
</dependency>
<dependency>
<groupId>net.openhft</groupId>
<artifactId>chronicle-quality-rules</artifactId>
<version>${chronicle-quality-rules.version}</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>checkstyle</id>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<configLocation>src/main/config/checkstyle.xml</configLocation>
<consoleOutput>true</consoleOutput>
<failOnViolation>true</failOnViolation>
<violationSeverity>warning</violationSeverity>
</configuration>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>${spotbugs.version}</version>
<dependencies>
<dependency>
<groupId>com.h3xstream.findsecbugs</groupId>
<artifactId>findsecbugs-plugin</artifactId>
<version>${findsecbugs.version}</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>spotbugs</id>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<effort>Max</effort>
<threshold>Low</threshold>
<failOnError>true</failOnError>
<excludeFilterFile>src/main/config/spotbugs-exclude.xml</excludeFilterFile>
<plugins>
<plugin>
<groupId>com.h3xstream.findsecbugs</groupId>
<artifactId>findsecbugs-plugin</artifactId>
<version>${findsecbugs.version}</version>
</plugin>
</plugins>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>${maven-pmd-plugin.version}</version>
<executions>
<execution>
<id>pmd</id>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<skipPmdError>false</skipPmdError>
<failOnViolation>true</failOnViolation>
<printFailingErrors>true</printFailingErrors>
<typeResolution>false</typeResolution>
<excludeFromFailureFile>src/main/config/pmd-exclude.properties</excludeFromFailureFile>
<excludes>
<!-- PMD-4578: Parser crashes on nested generics; drop once upstream fixes TextWire/YamlWireOut. -->
<exclude>**/TextWire.java</exclude>
<exclude>**/YamlWireOut.java</exclude>
</excludes>
</configuration>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>${jacoco-maven-plugin.version}</version>
<executions>
<execution>
<id>prepare-agent</id>
<goals>
<goal>prepare-agent</goal>
</goals>
<configuration>
<excludes>
<exclude>net/openhft/chronicle/wire/ReadmeChapter1Test*</exclude>
</excludes>
</configuration>
</execution>
<execution>
<id>report</id>
<phase>verify</phase>
<goals>
<goal>report</goal>
</goals>
</execution>
<execution>
<id>check</id>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
<configuration>
<rules>
<rule>
<element>BUNDLE</element>
<limits>
<limit>
<counter>LINE</counter>
<value>COVEREDRATIO</value>
<minimum>${jacoco.line.coverage}</minimum>
</limit>
<limit>
<counter>BRANCH</counter>
<value>COVEREDRATIO</value>
<minimum>${jacoco.branch.coverage}</minimum>
</limit>
</limits>
</rule>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<!-- Profile used when publishing sonar metrics -->
<profile>
<id>sonar</id>
Expand Down
Loading