DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://issues.apache.org/bugzilla/show_bug.cgi?id=41514>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 ------- Additional Comments From [EMAIL PROTECTED] 2007-02-12 09:02 ------- Hi Vincent, Thanks very much for taking the time to look closely at my patch. (In reply to comment #26) > (In reply to comment #24) > > (In reply to comment #23) > > > Adrian, thanks for doing this! I've looked at the patch and have a few comments > > > myself: > > > - I'd suggest to rename "strict-configuration" to "validate-configuration" (just > > > a personal preference). > > > > The configuration will still be validated regardless of the setting of this > > variable. Its just how the error is handled that makes the difference. If > > "strict-configuration" is true then FOP will immediately throw an exception > > and > > processing will terminate. If "strict-configuration" is set to false then > > FOP > > will log the error and attempt to continue parsing the configuration (if > > possible/meaningful). > > > > > - the name of the variable "strictFO" in > > > FopFactory.configure(Configuration) > > > seems wrong. There's nothing "FO" specific there. Furthermore, some "if > > > (strictFO)" should actually be "if (strictConfig)", right? > > > > I think you may have been looking at an older patch. The variable > > "strictValidation" is as before and the new variable is called > > "strictUserConfigValidation". > > No, Jeremias is right actually. There are 'if (strictFO)' statements to test > if > an exception has to be thrown. That should be 'if > (validateUserConfigStrictly())'. I should have better looked. Darn it.. I didn't think Jeremias was refering to the configure method. I missed this, thanks. > Also, in the new code a HyphenationTreeResolver is no longer created and > assigned to the hyphResolver field. That should inevitably lead to > a NullPointerException later, however the hyphenation junit tests pass. > Strange. > Are you sure of what you're doing? Thanks for checking this, I made a mistake refactoring here :-(. > One other thing: when getting the default-page-settings parameter it's not > necessary to test if pageConfig is null; it will never be. See the javadoc of > getChild. I've read the javadoc and have simplified/removed this unnecessary check now. > > > > > Adrian, would you please install the CheckStyle plug-in in your IDE? There are a > > > few nits about the Java style in your patch. Checkstyle will help you find them. > > Something checkstyle doesn't seem to be catching: we usually don't put spaces > inside brackets: if (test) instead of if ( test ). Yes I have noticed this recently and believe all the code I've added adheres to this now. > > > > > > Get well quickly, Adrian! > > > > After a weekend in bed am feeling much better today thanks. I had installed > > checksytle but not enabled it! ;-( I will recreate the patch this morning. > > Hope you feel better now! > > Sorry, I'll ask you to create yet another patch, but that should really be > the > last one! Its ok, I am learning all the bumps for fop patch submissions - hopefully should be smoother for you guys next time :-). Kind regards, Adrian. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
