-
Notifications
You must be signed in to change notification settings - Fork 113
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.
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
Supporting Jakarta Faces 3.0 in Config Files #4726
Conversation
Signed-off-by: Gurkan Erdogdu <cgurkanerdogdu@gmail.com>
Signed-off-by: Gurkan Erdogdu <cgurkanerdogdu@gmail.com>
The xsd files checked-in contain xi:include tags which are part of the source xsds. These xsds will not load properly because the .inc files are missing. Please use the published xsd available at https://jakarta.ee/xml/ns/jakartaee/ |
Signed-off-by: Gurkan Erdogdu <cgurkanerdogdu@gmail.com>
Hi @hussainnm |
// the systemId. | ||
|
||
// If no system ID, defer to superclass | ||
if (systemId == null) { | ||
InputSource result; | ||
try { | ||
result = super.resolveEntity(publicId, null); | ||
} catch (IOException | SAXException e) { | ||
} | ||
catch (IOException | SAXException e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as I initially disliked the catch being on the same line of the closing brace, this is actually according to the Sun standard and the Jakarta EE standard (which inherits from Sun)
* @param validating whether or not we're validating | ||
* @param documentURI a URL to the configuration resource to be parsed | ||
* @throws Exception general error | ||
* @param servletContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a tiny nit and the rest of the Mojarra code is certainly not compliant, but the Jakarta EE coding standards ask to not indent javadoc. Its same line, or when too large (more than 120 chars) beginning of next line.
@@ -78,6 +77,7 @@ | |||
|
|||
private static final String JAVAEE_SCHEMA_LEGACY_DEFAULT_NS = "http://java.sun.com/xml/ns/javaee"; | |||
private static final String JAVAEE_SCHEMA_DEFAULT_NS = "http://xmlns.jcp.org/xml/ns/javaee"; | |||
private static final String JAKARTAEE_SCHEMA_DEFAULT_NS="https://jakarta.ee/xml/ns/jakartaee"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost too tiny to remark, but the coding standard asks for a space left and right of the assignment,
} else { | ||
returnDoc = (Document) domSource.getNode(); | ||
returnDoc = ((Document) domSource.getNode()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extra braces are not needed here, just (Document) domSource.getNode()
is enough
final int size = 1024; | ||
byte[] b = new byte[size]; | ||
String s; | ||
while (!isZeroLengthOrEmpty && -1 != is.read(b, 0, size)) { | ||
s = new String(b, RIConstants.CHAR_ENCODING).trim(); | ||
s = (new String(b, RIConstants.CHAR_ENCODING)).trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the extra braces here.
Thanks a lot @gerdogdu! I provided some feedback on the formatting only, but they are very small nits and don't stand in the way of merging this. Thanks again! :) |
Your welcome @arjantijms . Actually I have been using Jakarta EE based Eclipse formatter. |
@gerdogdu thanks again, no worries about the formatting ;) |
@@ -412,7 +455,7 @@ private DocumentBuilder getNonValidatingBuilder() throws Exception { | |||
} | |||
|
|||
private DocumentBuilder getBuilderForSchema(Schema schema) throws Exception { | |||
factory = DbfFactory.getFactory(); | |||
this.factory = DbfFactory.getFactory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this this.
was added here and not in other places?
No description provided.