[Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]

[Atsushi Eno]

If your commit involved MANY changes, then they should be split anyways.

Even only with that point, I'm pretty much tempted to revert your
changes.

Atsushi Eno

Gert Driesen wrote:
> Atsushi,
> 
> This change is necessary for compatibility with MS and should not affect
> performance at all.
> 
> Lazy initialization of ClientConfigurationSystem actually improves startup
> time, and fixes the t28 standalone test.
> 
> It also removes the dependency on internal hacks (which were necessary for
> the 1.0 profile), and instead relies on the implementation of
> IConfigErrorInfo for retrieving filename/linenumber info.
> 
> My changes do not block future performance improvements. You'll just have to
> implement IConfigErrorInfo on your XmlNodeReader-based implementation.
> 
> I wouldn't see why reverting this patch is necessary as it:
> 
> * improves compatibility with MS
> * adds unit tests to verify this
> * fixes a few minor issues
> 
> while still allowing for a different internal implementation.
> 
> If any patch with such characteristics would be removed, then nothing much
> would be left ;-)
> 
> Gert
> 
> -----Original Message-----
> From: mono-devel-list-bounces at lists.ximian.com
> [mailto:mono-devel-list-bounces at lists.ximian.com] On Behalf Of Atsushi Eno
> Sent: donderdag 26 juni 2008 12:51
> To: 'mono-devel-list'
> Subject: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
> trunk/mcs/class/System.Configuration: . System.Configuration
> Test/System.Configuration Test/standalone]
> 
> Man, I don't think this is a good change. We (in Boston) are talking
> about sys.configuration optimization, and will likely have to eliminate
> XmlTextReader dependency (actually I have such a change possibly
> to switch to XmlNodeReader at some stage). Your change would make
> performance worse, or at least block significant performance
> improvements.
> 
> I see almost no benefits and will revert unless you claim very
> important improvements.
> 
> Atsushi Eno
> 
> 
> -------- Original Message --------
> Subject: [Mono-patches] r106626 - in 
> trunk/mcs/class/System.Configuration: .	System.Configuration 
> Test/System.Configuration Test/standalone
> Date: Thu, 26 Jun 2008 06:31:08 -0400 (EDT)
> From: Gert Driesen (gert.driesen at pandora.be) 
> <mono-patches-list at lists.ximian.com>
> To: mono-patches at lists.ximian.com, ximian.monolist at gmail.com, 
> mono-svn-patches-garchive-20758 at googlegroups.com
> 
> Author: gert
> Date: 2008-06-26 06:31:07 -0400 (Thu, 26 Jun 2008)
> New Revision: 106626
> 
> Added:
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ConfigXmlTextReade
> r.cs
>  
> trunk/mcs/class/System.Configuration/Test/System.Configuration/Configuration
> ErrorsExceptionTest.cs
>     trunk/mcs/class/System.Configuration/Test/standalone/Assert.cs
> Modified:
>     trunk/mcs/class/System.Configuration/ChangeLog
>     trunk/mcs/class/System.Configuration/System.Configuration.dll.sources
>  
> trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
> .cs
>     trunk/mcs/class/System.Configuration/System.Configuration/ChangeLog
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
> nSystem.cs
>     trunk/mcs/class/System.Configuration/System.Configuration/ConfigInfo.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration/Configuration.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ConfigurationEleme
> nt.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ConfigurationError
> sException.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ConfigurationLocat
> ion.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration/ConfigurationSecti
> on.cs
>  
> trunk/mcs/class/System.Configuration/System.Configuration_test.dll.sources
>     trunk/mcs/class/System.Configuration/Test/System.Configuration/ChangeLog
>     trunk/mcs/class/System.Configuration/Test/standalone/ChangeLog
>     trunk/mcs/class/System.Configuration/Test/standalone/Makefile
>     trunk/mcs/class/System.Configuration/Test/standalone/t28.cs
>     trunk/mcs/class/System.Configuration/Test/standalone/t42.cs
> Log:
> * ConfigurationErrorsExceptionTest.cs: Added tests for ctors and
> GetFilename/GetLineNumber overloads.
> * System.Configuration_test.dll.sources: added
> ConfigurationErrorsExceptionTest.cs.
> * System.Configuration.dll.sources: added ConfigXmlTextReader.cs.
> * ConfigurationElement.cs: Use ConfigurationErrorsException instead of
> ConfigurationException, and pass reader to ConfigurationErrorsException
> ctor to allow for file/linenumber info in exception message.
> * ConfigurationErrorsException.cs: Removed local bareMessage field, and
> use base.BareMessage field instead. Fixed Message property to only add
> filename if not null or zero-length string, and only add line if not
> zero. In GetFilename/GetLineNumber overloads, only try to get info if
> node/reader implements IConfigErrorInfo.
> * ConfigurationSection.cs: Use ConfigXmlTextReader instead of
> XmlTextReader to allow for file/linenumber info in exception messages.
> * ConfigXmlTextReader.cs: Added XmlTextReader that implements
> IConfigErrorInfo.
> * ConfigurationLocation.cs: Use ConfigXmlTextReader instead of
> XmlTextReader to allow for file/linenumber info in exception messages.
> * ClientConfigurationSystem.cs: Perform lazy initialization and
> wrap exceptions in ConfigurationErrorsException. Fixes standalone test
> t28.
> * Configuration.cs: Use ConfigXmlTextReader instead of XmlTextReader
> to allow for file/linenumber info in exception messages.
> * ConfigInfo.cs: Fixed ThrowException to use file/number info available
> in XmlTextReader for exception message.
> * AppSettingsSection.cs: Use ConfigXmlTextReader instead of
> XmlTextReader to allow for file/linenumber info in exception messages.
> * Assert.cs: added.
> * Makefile: added Assert.cs, and explicitly specify output file.
> * t28.cs: Improved test.
> 
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 
>