Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Atsushi,
This is not making much sense. Just about every member/class in
System.Xml.Serialization is marked like that.
Developers using that (eg. for our web services stack) must then indeed be
real idiots.
Marking a class or member as .NET Framework infrastructure means you
should avoid using it in user code as its behavior is not documented and
subject to changes. However, it's of course perfectly valid to use those
classes from within the .NET/Mono Framework infrastructure.
In this specific case we're talking about an interface that by itself does
not have any logic. But should the usage of that interface related to
ConfigurationErrorsException change, than my unit tests will allow us to
notice that.
With my change, my goal was not just compatibility with MS. The only reason
why - in this case - compatibility was nice to have, was to get unit tests
that pass on both Mono and MS.
I can't see why implementing a public interface is worse than using an
internal interface in combination with checks for a specific class for the
exact same purpose?
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno
Sent: vrijdag 27 juni 2008 2:37
To: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
Actually anyone who uses this IConfigErrorInfo is an idiot because
it is explicitly written as .NET FX internal.
This API supports the .NET Framework infrastructure and is not
intended to be used directly from your code.
So, compatibility is COMPLETELY no value here.
You don't have to flood mono-dev next time. I will silently
eliminate extraneous changes. I really don't care about those
pointless compatibility and will be glad to eliminate.
Atsushi Eno
Gert Driesen wrote:
Atsushi,
I'm not deliberately ignoring your or anyone else's advice. Perhaps my
idea
of trivial changes is different than yours, definitely if a very large
part
of those changes is covered by unit tests. Next time, I'll flood mono-dev
with small patches ;-)
I'm definitely not claiming my changes are more important than performance
improvements, but I'd be surprised if both can't be combined.
Gert
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: donderdag 26 juni 2008 19:58
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
In short, you are not going to change your commit policy right?
When I go ahead and make significant refactoring, I'll clean your
changes up and mark insignificant compatibility tests as Ignore.
Insignificant compatibility mastur* should not block significant
performance improvements.
Atsushi Eno
Gert Driesen wrote:
Hey Atsushi,
Apart from the change in ClientConfigurationSystem my patch was about
using
the IConfigErrorInfo interface for getting filename/linenumber info
instead
of explicitly checking for XmlTextReader, or using internal
IConfigXmlNode
interface.
Before the introduction of IConfigErrorInfo, we had to resort to this. My
changes just allow us to use this new interface, which - at the same time
-
allows us to write unit tests for this (as this now matches the MS
implementation).
I don't intend to write a book about these trivial changes, but I've
responded to your comments inline.
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi
Eno
Sent: donderdag 26 juni 2008 17:41
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
No. You claim as if your change were only about one issue, which it
NOT true. Here is concrete description:
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -73,7 +73,7 @@
if (File != ) {
try {
Stream s = System.IO.File.OpenRead
(File);
- XmlReader subreader = new
XmlTextReader (s);
+ XmlReader subreader = new
ConfigXmlTextReader (s, File);
base.DeserializeElement (subreader,
serializeCollectionKey);
s.Close ();
}
For example it is about
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Gert, On Thu, Jun 26, 2008 at 12:03 PM, Gert Driesen [EMAIL PROTECTED] wrote: I'm not deliberately ignoring your or anyone else's advice. Perhaps my idea of trivial changes is different than yours, definitely if a very large part of those changes is covered by unit tests. Next time, I'll flood mono-dev with small patches ;-) For what it's worth, have you considered creating a local GIT repository that mirrors the SVN repository and use this to make local atomic commits as suggested by Jb, but doing so in a way that allows you to make a larger grouped commit to SVN such that you can retain the commit history? This would also allow nicely for creating local branches specific to each extended or non-mainstream feature as well as related unit tests you might be working on and, when you feel its potentially ready for commit, submit a patch to the dev list for discussion. This ensures that the primary maintainer of the given code maintains control while at the same time allowing you the ability to move forward with your changes via your local branch if for some reason it's felt the changes are not appropriate for the main branch. I assume you've seen the reference on how to go about the git-svn process for the Mono SVN repository, but just in case http://www.mono-project.com/GitSVN -- /M:D M. David Peterson Co-Founder Chief Architect, 3rd Urban, LLC Email: [EMAIL PROTECTED] | [EMAIL PROTECTED] Mobile: (206) 999-0588 http://3rdandUrban.com | http://amp.fm | http://www.oreillynet.com/pub/au/2354 ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
I didn't send it immediately after I wrote it (as the hotel was disconnected) and anyways I didn't feel I have to reply to this fruitless post immediately. But as you requested, here it is. Gert Driesen wrote: This is not making much sense. Just about every member/class in System.Xml.Serialization is marked like that. Developers using that (eg. for our web services stack) must then indeed be real idiots. Yes they are idiots, as long as they depend on internals. You are trying to imply that I meant EVERYONE who use XmlSerializer is idiot, but that's totally wrong. Marking a class or member as .NET Framework infrastructure means you should avoid using it in user code as its behavior is not documented and subject to changes. However, it's of course perfectly valid to use those classes from within the .NET/Mono Framework infrastructure. In this specific case we're talking about an interface that by itself does not have any logic. But should the usage of that interface related to ConfigurationErrorsException change, than my unit tests will allow us to notice that. With my change, my goal was not just compatibility with MS. The only reason why - in this case - compatibility was nice to have, was to get unit tests that pass on both Mono and MS. Yes your goal was just compatibility. The tests just asserts that. There is no additional value. Having compat tests for internal-only is bad because Microsoft is 1) either not ready to provide fixed API and likely change in the future, and/or 2) they do not provide enough documentation and that fact blocks us from implementing .net-compatible components. In such situation we rather choose quick functional and effective implementation that saves reasonable mortals. Sticking to compatibility does not save anyone and hence such people just leave extraneous tests. Having extraneous tests 1) blocks maintainers who are often responsible (unlike you) to go further development and 2) costs everyone extraneous time for patch reading. Hence it goes below the balance point. Well, probably it won't be understood by those who were never flooded with extraneous patches and were cost their precious time. I can't see why implementing a public interface is worse than using an internal interface in combination with checks for a specific class for the exact same purpose? Public or non-public is really insignificant here. Atsushi Eno ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
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: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] 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 ([EMAIL PROTECTED]) [EMAIL PROTECTED] To: [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED] 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
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno Sent: donderdag 26 juni 2008 13:41 To: Gert Driesen Cc: 'mono-devel-list' Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone] If your commit involved MANY changes, then they should be split anyways. The only change that could be split is the lazy init fix for ClientConfigurationSystem. Even only with that point, I'm pretty much tempted to revert your changes. Yeah, I'm glad my (any?) contributions are that much appreciated. Gert 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: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] 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 ([EMAIL PROTECTED]) [EMAIL PROTECTED] To: [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED] 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
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Even only with that point, I'm pretty much tempted to revert your changes. Yeah, I'm glad my (any?) contributions are that much appreciated. Come on Gert, it's definitely not the first time that you're told that your commits are: * totally not atomic, * mixing totally different concerns, And for some of us that keep an eye on the code coming in, it makes that task much harder. That doesn't mean that we don't appreciate contributions. But once again, I already told you that I was not happy with the way you're making commits, and am not the first one. And I can certainly understand the maintainer frustration to see this huge changeset coming in. -- Jb Evain [EMAIL PROTECTED] ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Hey Jb, Sorry dude, but this wasn't a large changeset. If the definition of a large changeset is a patch which adds a large set of unit tests, then I'm guilty as charged. Apart from removing a few extra tabs (my mistake), everything I changed is documented in the changelog and covered by unit tests or standalone tests. There's only one part of the patch that could be committed separately, and this is the change to ClientConfigurationSystem. And again, this change fixes a failing standalone test (t28). Please be reasonable. What more can you ask? Gert -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Jb Evain Sent: donderdag 26 juni 2008 15:11 To: Gert Driesen Cc: Atsushi Eno; mono-devel-list Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone] Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Even only with that point, I'm pretty much tempted to revert your changes. Yeah, I'm glad my (any?) contributions are that much appreciated. Come on Gert, it's definitely not the first time that you're told that your commits are: * totally not atomic, * mixing totally different concerns, And for some of us that keep an eye on the code coming in, it makes that task much harder. That doesn't mean that we don't appreciate contributions. But once again, I already told you that I was not happy with the way you're making commits, and am not the first one. And I can certainly understand the maintainer frustration to see this huge changeset coming in. -- Jb Evain [EMAIL PROTECTED] ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
To: Gert Driesen Cc: Atsushi Eno; mono-devel-list Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone] Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Even only with that point, I'm pretty much tempted to revert your changes. Yeah, I'm glad my (any?) contributions are that much appreciated. Come on Gert, it's definitely not the first time that you're told that your commits are: * totally not atomic, * mixing totally different concerns, And for some of us that keep an eye on the code coming in, it makes that task much harder. That doesn't mean that we don't appreciate contributions. But once again, I already told you that I was not happy with the way you're making commits, and am not the first one. And I can certainly understand the maintainer frustration to see this huge changeset coming in. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Sorry dude, but this wasn't a large changeset. Alright, so because you simply *refuse* to hear what we telling you, let me rephrase and explain. If the definition of a large changeset is a patch which adds a large set of unit tests, then I'm guilty as charged. Apart from removing a few extra tabs (my mistake), everything I changed is documented in the changelog and covered by unit tests or standalone tests. It's not because changes are covered by unit tests and described in the ChangeLog that changes are *atomic*. We're programming right, so we believe in the separation of concerns? I (at least try) to apply the separation of concerns in commits as well. I like to think to a SCM as an important developer tool, and I'm not the only one to monitor patches coming in. By having small self contained patch, it's much easier to navigate through history, detect defects, and revert them precisely. Am sorry that you don't see the issue, but there's one. Really. And not only that allow us to be aware of the code coming in, and to work on it if needed, but it also allow us to review the code, we, maintainers, that are asked to maintain a piece of code. By checking patches in the way you're doing it, you're simply preventing us to do this work efficiently. And as I said, it's not because you're adding tests and writing ChangeLog entries that some mistakes are not buried into the hundred of lines of changes. We already saw that happen didn't we? To sum up, you're checking in large set of tests + behavior changes + style changes all in the same commit. And that's not a good practice, at all. Please be reasonable. What more can you ask? Please listen when different people like Atsushi or Sébastien (or even me for that matters) repeatedly say the same thing. And I'd ask you to follow their advices. -- Jb Evain [EMAIL PROTECTED] ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Please be reasonable. What more can you ask? Is that worth noting that the particular commit we're talking about broke the build? -- Jb Evain [EMAIL PROTECTED] ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
I think that's kind of mistake we often make; CONFIGURATION_DEP annoyance. Let's not worry much about it ;-) Atsushi Eno Jb Evain wrote: Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Please be reasonable. What more can you ask? Is that worth noting that the particular commit we're talking about broke the build? ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Hey Atsushi,
Apart from the change in ClientConfigurationSystem my patch was about using
the IConfigErrorInfo interface for getting filename/linenumber info instead
of explicitly checking for XmlTextReader, or using internal IConfigXmlNode
interface.
Before the introduction of IConfigErrorInfo, we had to resort to this. My
changes just allow us to use this new interface, which - at the same time -
allows us to write unit tests for this (as this now matches the MS
implementation).
I don't intend to write a book about these trivial changes, but I've
responded to your comments inline.
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno
Sent: donderdag 26 juni 2008 17:41
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
No. You claim as if your change were only about one issue, which it
NOT true. Here is concrete description:
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -73,7 +73,7 @@
if (File != ) {
try {
Stream s = System.IO.File.OpenRead
(File);
- XmlReader subreader = new
XmlTextReader (s);
+ XmlReader subreader = new
ConfigXmlTextReader (s, File);
base.DeserializeElement (subreader,
serializeCollectionKey);
s.Close ();
}
For example it is about ConfigXmlTextReader change.
= The use of ConfigXmlTextReader is necessary as it implements
IConfigErrorInfo. I consider this part of the same change.
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -32,28 +32,39 @@
using System.Reflection;
using System.Configuration.Internal;
-namespace System.Configuration {
-
+namespace System.Configuration
+{
internal class ClientConfigurationSystem : IInternalConfigSystem
{
- readonly Configuration cfg;
+ Configuration cfg;
- public ClientConfigurationSystem () {
- Assembly a = Assembly.GetEntryAssembly();
- string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
-
- if (a == null exePath == null)
- exePath = ;
-
- cfg =
ConfigurationManager.OpenExeConfigurationInternal
(ConfigurationUserLevel.None, a, exePath);
+ public ClientConfigurationSystem ()
+ {
}
+ private Configuration Configuration {
+ get {
+ if (cfg == null) {
+ Assembly a =
Assembly.GetEntryAssembly();
+ string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
+
+ if (a == null exePath == null)
+ exePath = string.Empty;
+
+ try {
+ cfg =
ConfigurationManager.OpenExeConfigurationInternal (
+
ConfigurationUserLevel.None, a, exePath);
+ } catch (Exception ex) {
+ throw new
ConfigurationErrorsException (Error Initializing the configuration
system., ex);
+ }
+ }
+ return cfg;
+ }
+ }
+
object IInternalConfigSystem.GetSection (string configKey)
{
- if (cfg == null)
- return null;
-
- ConfigurationSection s = cfg.GetSection (configKey);
+ ConfigurationSection s = Configuration.GetSection
(configKey);
return s != null ? s.GetRuntimeObject () : null;
}
This is about lazy
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Jb, I envy your positive attitude. Atsushi, that for the quick fix. I'll do a full bootstrap next. Gert -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Jb Evain Sent: donderdag 26 juni 2008 18:46 To: Gert Driesen Cc: Atsushi Eno; mono-devel-list Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone] Hey, On 6/26/08, Gert Driesen [EMAIL PROTECTED] wrote: Please be reasonable. What more can you ask? Is that worth noting that the particular commit we're talking about broke the build? -- Jb Evain [EMAIL PROTECTED] ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
In short, you are not going to change your commit policy right?
When I go ahead and make significant refactoring, I'll clean your
changes up and mark insignificant compatibility tests as Ignore.
Insignificant compatibility mastur* should not block significant
performance improvements.
Atsushi Eno
Gert Driesen wrote:
Hey Atsushi,
Apart from the change in ClientConfigurationSystem my patch was about using
the IConfigErrorInfo interface for getting filename/linenumber info instead
of explicitly checking for XmlTextReader, or using internal IConfigXmlNode
interface.
Before the introduction of IConfigErrorInfo, we had to resort to this. My
changes just allow us to use this new interface, which - at the same time -
allows us to write unit tests for this (as this now matches the MS
implementation).
I don't intend to write a book about these trivial changes, but I've
responded to your comments inline.
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno
Sent: donderdag 26 juni 2008 17:41
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
No. You claim as if your change were only about one issue, which it
NOT true. Here is concrete description:
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -73,7 +73,7 @@
if (File != ) {
try {
Stream s = System.IO.File.OpenRead
(File);
-XmlReader subreader = new
XmlTextReader (s);
+XmlReader subreader = new
ConfigXmlTextReader (s, File);
base.DeserializeElement (subreader,
serializeCollectionKey);
s.Close ();
}
For example it is about ConfigXmlTextReader change.
= The use of ConfigXmlTextReader is necessary as it implements
IConfigErrorInfo. I consider this part of the same change.
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs2008-06-26 10:31:07 UTC (rev 106626)
@@ -32,28 +32,39 @@
using System.Reflection;
using System.Configuration.Internal;
-namespace System.Configuration {
-
+namespace System.Configuration
+{
internal class ClientConfigurationSystem : IInternalConfigSystem
{
-readonly Configuration cfg;
+Configuration cfg;
-public ClientConfigurationSystem () {
-Assembly a = Assembly.GetEntryAssembly();
-string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
-
-if (a == null exePath == null)
-exePath = ;
-
-cfg =
ConfigurationManager.OpenExeConfigurationInternal
(ConfigurationUserLevel.None, a, exePath);
+public ClientConfigurationSystem ()
+{
}
+private Configuration Configuration {
+get {
+if (cfg == null) {
+Assembly a =
Assembly.GetEntryAssembly();
+string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
+
+if (a == null exePath == null)
+exePath = string.Empty;
+
+try {
+cfg =
ConfigurationManager.OpenExeConfigurationInternal (
+
ConfigurationUserLevel.None, a, exePath);
+} catch (Exception ex) {
+throw new
ConfigurationErrorsException (Error Initializing the configuration
system., ex);
+}
+}
+return cfg;
+}
+}
+
object IInternalConfigSystem.GetSection (string configKey)
{
-if (cfg
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Atsushi,
I'm not deliberately ignoring your or anyone else's advice. Perhaps my idea
of trivial changes is different than yours, definitely if a very large part
of those changes is covered by unit tests. Next time, I'll flood mono-dev
with small patches ;-)
I'm definitely not claiming my changes are more important than performance
improvements, but I'd be surprised if both can't be combined.
Gert
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: donderdag 26 juni 2008 19:58
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
In short, you are not going to change your commit policy right?
When I go ahead and make significant refactoring, I'll clean your
changes up and mark insignificant compatibility tests as Ignore.
Insignificant compatibility mastur* should not block significant
performance improvements.
Atsushi Eno
Gert Driesen wrote:
Hey Atsushi,
Apart from the change in ClientConfigurationSystem my patch was about
using
the IConfigErrorInfo interface for getting filename/linenumber info
instead
of explicitly checking for XmlTextReader, or using internal IConfigXmlNode
interface.
Before the introduction of IConfigErrorInfo, we had to resort to this. My
changes just allow us to use this new interface, which - at the same time
-
allows us to write unit tests for this (as this now matches the MS
implementation).
I don't intend to write a book about these trivial changes, but I've
responded to your comments inline.
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno
Sent: donderdag 26 juni 2008 17:41
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
No. You claim as if your change were only about one issue, which it
NOT true. Here is concrete description:
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -73,7 +73,7 @@
if (File != ) {
try {
Stream s = System.IO.File.OpenRead
(File);
-XmlReader subreader = new
XmlTextReader (s);
+XmlReader subreader = new
ConfigXmlTextReader (s, File);
base.DeserializeElement (subreader,
serializeCollectionKey);
s.Close ();
}
For example it is about ConfigXmlTextReader change.
= The use of ConfigXmlTextReader is necessary as it implements
IConfigErrorInfo. I consider this part of the same change.
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs2008-06-26 10:31:07 UTC (rev 106626)
@@ -32,28 +32,39 @@
using System.Reflection;
using System.Configuration.Internal;
-namespace System.Configuration {
-
+namespace System.Configuration
+{
internal class ClientConfigurationSystem : IInternalConfigSystem
{
-readonly Configuration cfg;
+Configuration cfg;
-public ClientConfigurationSystem () {
-Assembly a = Assembly.GetEntryAssembly();
-string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
-
-if (a == null exePath == null)
-exePath = ;
-
-cfg =
ConfigurationManager.OpenExeConfigurationInternal
(ConfigurationUserLevel.None, a, exePath);
+public ClientConfigurationSystem ()
+{
}
+private Configuration Configuration {
+get {
+if (cfg == null) {
+Assembly a =
Assembly.GetEntryAssembly();
+string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
+
+if (a == null exePath == null
Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in trunk/mcs/class/System.Configuration: . System.Configuration Test/System.Configuration Test/standalone]
Actually anyone who uses this IConfigErrorInfo is an idiot because
it is explicitly written as .NET FX internal.
This API supports the .NET Framework infrastructure and is not
intended to be used directly from your code.
So, compatibility is COMPLETELY no value here.
You don't have to flood mono-dev next time. I will silently
eliminate extraneous changes. I really don't care about those
pointless compatibility and will be glad to eliminate.
Atsushi Eno
Gert Driesen wrote:
Atsushi,
I'm not deliberately ignoring your or anyone else's advice. Perhaps my idea
of trivial changes is different than yours, definitely if a very large part
of those changes is covered by unit tests. Next time, I'll flood mono-dev
with small patches ;-)
I'm definitely not claiming my changes are more important than performance
improvements, but I'd be surprised if both can't be combined.
Gert
-Original Message-
From: Atsushi Eno [mailto:[EMAIL PROTECTED]
Sent: donderdag 26 juni 2008 19:58
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
In short, you are not going to change your commit policy right?
When I go ahead and make significant refactoring, I'll clean your
changes up and mark insignificant compatibility tests as Ignore.
Insignificant compatibility mastur* should not block significant
performance improvements.
Atsushi Eno
Gert Driesen wrote:
Hey Atsushi,
Apart from the change in ClientConfigurationSystem my patch was about
using
the IConfigErrorInfo interface for getting filename/linenumber info
instead
of explicitly checking for XmlTextReader, or using internal IConfigXmlNode
interface.
Before the introduction of IConfigErrorInfo, we had to resort to this. My
changes just allow us to use this new interface, which - at the same time
-
allows us to write unit tests for this (as this now matches the MS
implementation).
I don't intend to write a book about these trivial changes, but I've
responded to your comments inline.
Gert
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Atsushi Eno
Sent: donderdag 26 juni 2008 17:41
To: Gert Driesen
Cc: 'mono-devel-list'
Subject: Re: [Mono-dev] [Fwd: [Mono-patches] r106626 - in
trunk/mcs/class/System.Configuration: . System.Configuration
Test/System.Configuration Test/standalone]
No. You claim as if your change were only about one issue, which it
NOT true. Here is concrete description:
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/AppSettingsSection
.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -73,7 +73,7 @@
if (File != ) {
try {
Stream s = System.IO.File.OpenRead
(File);
- XmlReader subreader = new
XmlTextReader (s);
+ XmlReader subreader = new
ConfigXmlTextReader (s, File);
base.DeserializeElement (subreader,
serializeCollectionKey);
s.Close ();
}
For example it is about ConfigXmlTextReader change.
= The use of ConfigXmlTextReader is necessary as it implements
IConfigErrorInfo. I consider this part of the same change.
Modified:
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs
===
---
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs 2008-06-26 09:57:29 UTC (rev 106625)
+++
trunk/mcs/class/System.Configuration/System.Configuration/ClientConfiguratio
nSystem.cs 2008-06-26 10:31:07 UTC (rev 106626)
@@ -32,28 +32,39 @@
using System.Reflection;
using System.Configuration.Internal;
-namespace System.Configuration {
-
+namespace System.Configuration
+{
internal class ClientConfigurationSystem : IInternalConfigSystem
{
- readonly Configuration cfg;
+ Configuration cfg;
- public ClientConfigurationSystem () {
- Assembly a = Assembly.GetEntryAssembly();
- string exePath =
AppDomain.CurrentDomain.SetupInformation.ConfigurationFile;
-
- if (a == null exePath == null)
- exePath = ;
-
- cfg =
ConfigurationManager.OpenExeConfigurationInternal
(ConfigurationUserLevel.None, a, exePath);
+ public
