RAT-570: Create Reporter.Output object#697
Conversation
Claudenw
force-pushed
the
create-output-object
branch
from
6e82596 to
0fcd7da
Compare
|
Yes serde is serializer-deserializer
LinkedIn: http://www.linkedin.com/in/claudewarren
…On Mon 13 Jul 2026, 10:40 P. Ottlinger, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/ExclusionProcessor.java
<#697 (comment)>:
> @@ -74,7 +82,51 @@ public ExclusionProcessor() {
excludedCollections = new HashSet<>();
}
- /** Reset the ***@***.*** #lastMatcher} and ***@***.*** #lastMatcherBaseDir} to start again */
+ public Serde serde() {
What does Serde mean? Serialization/Deserialization?
—
Reply to this email directly, view it on GitHub
<#697?email_source=notifications&email_token=AASTVHUYDPUYJ375XVZOJIL5ESU7VA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRYGM2DSOJZHE22M4TFMFZW63VGMFZXG2LHN2SWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4683499995>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASTVHR5YEPA6DQZ4MTC6535ESU7VAVCNFSNUABDKJSXA33TNF2G64TZHM3DCNRZHAZDCO2JONZXKZJ3GQ4DCOJUG44TAMZWUF3AE>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
Remove unused code and use SiteRenderer instead of deprecated Renderer.
|
@ottlinger My plan is to fix the Maven and Ant issues when they get moved to separate projects. There will be lots of cleanup there as the code we are running for Maven is a bit old. |
ottlinger
left a comment
ottlinger
left a comment
There was a problem hiding this comment.
LGTM, except for the naming Serde. I'd prefer a named mapper class. WDYT?
| /** | ||
| * Serializes and deserializes the ExclusionProcessor to XML document | ||
| */ | ||
| public class Serde { |
There was a problem hiding this comment.
Could we rename this class as Serde does not seem to be a natural word ....
what about ExclusionProcessorXmlConverter or ExclusionProcessorXmlMapper?
There was a problem hiding this comment.
There are several cases where Serde is the name of the class that serializes and deserializes.
There are some common references:
- https://en.wikipedia.org/wiki/SerDes (though this one has an 's' at the end)
- Rust has a SerDe library.
- Hive has reference to several SerDe implementations: https://cwiki.apache.org/confluence/spaces/Hive/pages/27362059/SerDe
- Kafka streams uses them: https://kafka.apache.org/42/streams/developer-guide/datatypes/
- Spring uses serde: https://docs.spring.io/spring-cloud-stream/reference/kafka/kafka-streams-binder/record-serialization-and-deserialization.html
Several of these call the class/interface SerDes some are Serde. I feel that it is a common enough abbreviation that we can use it. I am open to changing the class name to SerDe or SerDes
There was a problem hiding this comment.
I'll rename the classes to SerDes then.
Should we change the builder methods as well or is SerDes.serde(); ok?
Personally I think SerDes.serDes(); looks strange to me.
Or would build() be clearer here or create()?
WDYT? @Claudenw
| /** Map of counter type to value */ | ||
| private final ConcurrentHashMap<ClaimStatistic.Counter, IntCounter> counterMap = new ConcurrentHashMap<>(); | ||
|
|
||
| public Serde serde() { |
There was a problem hiding this comment.
I'd prefer to rename this class as well as Serde is rather generic
There was a problem hiding this comment.
The class name is ClaimStatistic.Serde. In light of the comment above is this specific enough?
| reportables = new ArrayList<>(); | ||
| } | ||
|
|
||
| public Serde serde() { |
There was a problem hiding this comment.
I'd prefer a more specific class name as suggested above.
| return sources; | ||
| } | ||
|
|
||
| // for testing access |
There was a problem hiding this comment.
Is there something like Guava's @VisibleForTesting somewhere available in Commons? The more I read these comments, the more I prefer this annotation ;)
There was a problem hiding this comment.
We could add the guava library to pick up the @VisibleForTesting. But I think we should do that as a single PR.
| * Tests for StandardXmlFactory. | ||
| */ | ||
| class StandardXmlFactoryTests { | ||
| public class StandardXmlFactoryTests { |
There was a problem hiding this comment.
Is the visibility change important for future PRs?
There was a problem hiding this comment.
the simpleDocument method is used in OutputTest. If hte class is not public the OutputTest can not find the method.
|
2 participants
This change requires RAT-568 (now merged)
This change is to defines a
Reporter.Outputclass that contains the configuration that was used for the execution, the XML document that was produced, and the ClaimStatistic that captures the numbers various types of files, licenses, license categories and other statistics from the run.The
Reporter.execute()will create aReporter.Outputand will not automatically format the output as specified in the configuration. Formatting output becomes a second step.Reporter.Outputwill have 2 methods to format the outputformat(ReportConfiguration)- will use the output and stylesheet defined in the configuration to format the output.format(IOSupplier stylesheet, IOSupplier output)- will apply an arbitrary stylesheet against the contained document and send the output to the location specified.This change will make it easier for the Maven version to generate the document in the Mojo.check class and the final output in the Mojo.report class without regenerating the report.
The
Reporter.Outputwill be immutable and constructed using a builder pattern.The builder will be able to read the 3 components (Configuration, XML document, and ClaimStatistic) from input streams. Each of those components will be extended to include a Serde implementation to write to, and restore from and IO stream.