elharo commented on code in PR #25: URL: https://github.com/apache/maven-xinclude-extension/pull/25#discussion_r2014870458
########## src/main/java/org/apache/maven/xinclude/stax/XIncludeStreamReader.java: ########## @@ -70,9 +71,19 @@ @SuppressWarnings("checkstyle:MissingSwitchDefault") class XIncludeStreamReader extends StreamReaderDelegate { - private static final String XINCLUDE_NAMESPACE = "http://www.w3.org/2001/XInclude"; - private static final String XINCLUDE_INCLUDE = "include"; - private static final String XINCLUDE_FALLBACK = "fallback"; + public static final String XINCLUDE_NAMESPACE = "http://www.w3.org/2001/XInclude"; + public static final String XINCLUDE_INCLUDE_ELEMENT = "include"; + public static final String XINCLUDE_FALLBACK_ELEMENT = "fallback"; + public static final String XINCLUDE_LOCAL_ATTRIBUTES = "http://www.w3.org/2001/XInclude/local-attributes"; + public static final String XINCLUDE_HREF_ATTRIBUTE = "href"; + public static final String XINCLUDE_PARSE_ATTRIBUTE = "parse"; + public static final String XINCLUDE_XPOINTER_ATTRIBUTE = "xpointer"; + public static final String XINCLUDE_FRAGID_ATTRIBUTE = "fragid"; + public static final String XINCLUDE_SET_XML_ID_ATTRIBUTE = "set-xml-id"; Review Comment: I'm not sure where this one is coming from. I've never seen it before. is there a spec that mentions this? Google didn't turn one up. ########## src/main/java/org/apache/maven/xinclude/stax/XIncludeStreamReader.java: ########## @@ -70,9 +71,19 @@ @SuppressWarnings("checkstyle:MissingSwitchDefault") class XIncludeStreamReader extends StreamReaderDelegate { - private static final String XINCLUDE_NAMESPACE = "http://www.w3.org/2001/XInclude"; - private static final String XINCLUDE_INCLUDE = "include"; - private static final String XINCLUDE_FALLBACK = "fallback"; + public static final String XINCLUDE_NAMESPACE = "http://www.w3.org/2001/XInclude"; Review Comment: Maybe this one. The rest are not helpful. Named constants clarify numbers like 0.75 but they are not better than literal strings and usually worse. For instance "include" is much simpler, clearer, and more obvious than XINCLUDE_INCLUDE_ELEMENT. Also note that "include" is not an element but rather an element name. -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org