commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 34362] - [configuration] AbstractFileConfiguration.save() creates a new file instead of overwritting the existing one
Date Fri, 08 Apr 2005 06:09:53 GMT
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 oliver.heger@t-online.de  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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message