https://issues.apache.org/bugzilla/show_bug.cgi?id=46791
Helder Magalhães <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |PatchAvailable CC| |[email protected] --- Comment #1 from Helder Magalhães <[email protected]> 2009-03-03 01:37:12 PST --- Hi Daniel, Nice to see your contribution. ;-) Without much knowledge on Batik's internals, I'm only reviewing the patch in terms of coding principles (at least for now). > import org.apache.batik.dom.svg.SVGGraphicsElement; > - > import org.w3c.dom.Node; This apparently redundant line separating (java from batik from w3c) includes is expected. See, for example, "sources/org/apache/batik/bridge/BaseScriptingEnvironment.java". > protected Node newNode() { > - return new BindableElement(null, null, namespaceURI, localName); > + return new BindableElement(); Indenting issue, please use 4 spaces for consistency. > - /** > + /** Indenting issue, please use 4 spaces instead of tab character. By the way, patches can be acknowledged by adding the "PatchAvailable" keyword to the report (which I've done). Adding the word to the report's title is also commonly seen, and I believe using both won't hurt (although I can be wrong). ;-) Finally, I'm not sure if the patch should include a modification to the "CHANGES" file or if this is to be addressed by whoever commits it (although I'd assume the latter). Regards, Helder Magalhães -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
