ottlinger commented on code in PR #697:
URL: https://github.com/apache/creadur-rat/pull/697#discussion_r3572189013
##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/ExclusionProcessor.java:
##########
@@ -337,4 +395,91 @@ private void extractPaths(final MatcherSet.Builder
matcherBuilder) {
}
}
+ /**
+ * Serializes and deserializes the ExclusionProcessor to XML document
+ */
+ public class Serde {
Review Comment:
Could we rename this class as Serde does not seem to be a natural word ....
what about ExclusionProcessorXmlConverter or ExclusionProcessorXmlMapper?
##########
apache-rat-core/src/main/java/org/apache/rat/ReportConfiguration.java:
##########
@@ -183,6 +196,10 @@ public ReportConfiguration() {
reportables = new ArrayList<>();
}
+ public Serde serde() {
Review Comment:
I'd prefer a more specific class name as suggested above.
##########
apache-rat-core/src/main/java/org/apache/rat/ReportConfiguration.java:
##########
@@ -242,6 +259,16 @@ public ReportableListWalker.Builder getSources() {
return builder;
}
+ // for testing access
+ Iterable<File> sources() {
+ return sources;
+ }
+
+ // for testing access
Review Comment:
Is there something like Guava's @VisibleForTesting somewhere available in
Commons? The more I read these comments, the more I prefer this annotation ;)
##########
apache-rat-core/src/main/java/org/apache/rat/report/claim/ClaimStatistic.java:
##########
@@ -113,6 +123,9 @@ public String displayName() {
/** Map of counter type to value */
private final ConcurrentHashMap<ClaimStatistic.Counter, IntCounter>
counterMap = new ConcurrentHashMap<>();
+ public Serde serde() {
Review Comment:
I'd prefer to rename this class as well as Serde is rather generic
##########
apache-rat-core/src/test/java/org/apache/rat/utils/StandardXmlFactoryTests.java:
##########
@@ -18,22 +18,48 @@
*/
package org.apache.rat.utils;
+import org.apache.rat.report.xml.writer.XmlWriter;
import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.xml.sax.SAXException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
import java.io.InputStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for StandardXmlFactory.
*/
-class StandardXmlFactoryTests {
+public class StandardXmlFactoryTests {
Review Comment:
Is the visibility change important for future PRs?
--
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]
