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=34362>. 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=34362 ------- Additional Comments From [EMAIL PROTECTED] 2005-04-08 08:09 ------- Jamie, thanks for your patch. Some comments: - A very important point are unit tests. If you can provide a test case that fails before the patch is applied and runs afterwards, this will demonstrate the problem and show that it is indeed solved. - Shouldn't the line File file = new File(url.getPath()); be replaced by File file = ConfigurationUtils.fileFromURL(url); ? Otherwise the test for null won't make sense. - There are some coding conventions, e.g. that curly brackets are placed on a new line. Please ensure that your code looks like the other code around it. - And a more general point: I think the root cause for the problems is that the location of the config file as it was resolved in the load() method is not stored. Using the same resolving logic again in the save() method will likely result in the same location, but this is not guaranteed. So I am not sure whether this patch will solve all problems. A while ago we had a discussion about redesigning the whole file resolving logic by introducing Locator objects, which implement specific algorithms for finding files (e.g. from a URL, from the classpath, etc.). But IIRC we didn't find a consensus then. Maybe it's time to start this discussion again? -- 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. --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
