[Mono-dev] [Mono-patches] r106777 - in trunk/mcs/class/System: System.ComponentModel Test/System.ComponentModel

Gert Driesen gert.driesen at telenet.be
Fri Jun 27 19:04:23 EDT 2008


Atsushi,

It's not worth discussing the change of argument name in an exception.

We have two choices:

* Either we revert your changes, and have our tests pass on both MS and
Mono.
* We keep your change, but remove the ParamName checks in the unit tests.

I'm ok with either. Simple as that.

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: zaterdag 28 juni 2008 0:57
To: 'mono-devel-list'
Subject: Re: [Mono-dev] [Mono-patches] r106777 - in trunk/mcs/class/System:
System.ComponentModel Test/System.ComponentModel

I'd rather wait for others' evaluation on whether your code had better 
be kept
or not. It is obvious to me though.

The practical issue with your change is that, what you likely did was 
just to
do "run-test-ondotnet" and changed the code that is very relevant, without
actually seeing or understanding what you changed there. That's not very 
good
way to "fix" code or write tests.

(I guess it as above, because you dare added tests to verify it; until I 
found
that fact I thought that is is because of easy copypasting. I even told some
other guys to not take this mistake seriously, as everyone makes such ones.)

Atsushi Eno

> Atsushi,
>
> Yes, the argument name in the ArgumentNullException does not match the
name
> of the method argument.
>
> This is probably because the argument check itself is not performed in the
> Find method, but bubbled up.
>
> This is a detail not worth discussing, but also one that was not worth
> reverting.
>
> 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: vrijdag 27 juni 2008 23:47
> To: Gert Driesen
> Cc: 'mono-devel-list'
> Subject: Re: [Mono-dev] [Mono-patches] r106777 - in
trunk/mcs/class/System:
> System.ComponentModel Test/System.ComponentModel
>
> Um, you're right, sadly.
>
> Then; check the name of the argument.
>
> Atsushi Eno
>
>> Atsushi,
>>
>> I did not ask you privately.
>>
>> Gert
>>
>> -----Original Message-----
>> From: Atsushi Eno [mailto:atsushi at ximian.com] 
>> Sent: vrijdag 27 juni 2008 23:34
>> To: Gert Driesen
>> Subject: Re: [Mono-patches] r106777 - in trunk/mcs/class/System:
>> System.ComponentModel Test/System.ComponentModel
>>
>> I never give any replies privately. Though if you ask it on any public 
>> list, you
>> might end up to feel ashamed, so I wouldn't recommend it.
>>
>>   
>>> Atsushi,
>>>
>>> Your changes introduce test failures on MS.
>>>
>>> Please explain why you need to revert that change!
>>>
>>> Gert
>>>
>>> -----Original Message-----
>>> From: mono-patches-bounces at lists.ximian.com
>>> [mailto:mono-patches-bounces at lists.ximian.com] On Behalf Of Atsushi
>>>     
>> Enomoto
>>   
>>> (atsushi at ximian.com)
>>> Sent: vrijdag 27 juni 2008 22:56
>>> To: mono-patches at lists.ximian.com; ximian.monolist at gmail.com;
>>> mono-svn-patches-garchive-20758 at googlegroups.com
>>> Subject: [Mono-patches] r106777 - in trunk/mcs/class/System:
>>> System.ComponentModel Test/System.ComponentModel
>>>
>>> Author: atsushi
>>> Date: 2008-06-27 16:56:26 -0400 (Fri, 27 Jun 2008)
>>> New Revision: 106777
>>>
>>> Modified:
>>>    trunk/mcs/class/System/System.ComponentModel/ChangeLog
>>>  
>>>
>>>     
>
trunk/mcs/class/System/System.ComponentModel/PropertyDescriptorCollection.cs
>>   
>>>    trunk/mcs/class/System/Test/System.ComponentModel/ChangeLog
>>>  
>>>
>>>     
>
trunk/mcs/class/System/Test/System.ComponentModel/PropertyDescriptorCollecti
>>   
>>> onTests.cs
>>> Log:
>>> too silly to document.
>>>
>>>
>>>
>>> Modified: trunk/mcs/class/System/System.ComponentModel/ChangeLog
>>> ===================================================================
>>> --- trunk/mcs/class/System/System.ComponentModel/ChangeLog	2008-06-27
>>> 20:45:49 UTC (rev 106776)
>>> +++ trunk/mcs/class/System/System.ComponentModel/ChangeLog	2008-06-27
>>> 20:56:26 UTC (rev 106777)
>>> @@ -1,3 +1,7 @@
>>> +2008-06-27  Atsushi Enomoto  <atsushi at ximian.com>
>>> +
>>> +	* PropertyDescriptorCollection.cs : huh.
>>> +
>>>  2008-06-27  Gert Driesen  <drieseng at users.sourceforge.net>
>>>  
>>>  	* EventDescriptorCollection.cs: Fixed Empty to return read-only
>>>
>>> Modified:
>>>
>>>     
>
trunk/mcs/class/System/System.ComponentModel/PropertyDescriptorCollection.cs
>>   
>>> ===================================================================
>>> ---
>>>
>>>     
>
trunk/mcs/class/System/System.ComponentModel/PropertyDescriptorCollection.cs
>>   
>>> 2008-06-27 20:45:49 UTC (rev 106776)
>>> +++
>>>
>>>     
>
trunk/mcs/class/System/System.ComponentModel/PropertyDescriptorCollection.cs
>>   
>>> 2008-06-27 20:56:26 UTC (rev 106777)
>>> @@ -140,7 +140,7 @@
>>>  		public virtual PropertyDescriptor Find (string name, bool
>>> ignoreCase)
>>>  		{
>>>  			if (name == null)
>>> -				throw new ArgumentNullException ("key");
>>> +				throw new ArgumentNullException ("name");
>>>  
>>>  			foreach (PropertyDescriptor p in properties) {
>>>  #if NET_2_0
>>>
>>> Modified: trunk/mcs/class/System/Test/System.ComponentModel/ChangeLog
>>> ===================================================================
>>> --- trunk/mcs/class/System/Test/System.ComponentModel/ChangeLog
>>>     
>> 2008-06-27
>>   
>>> 20:45:49 UTC (rev 106776)
>>> +++ trunk/mcs/class/System/Test/System.ComponentModel/ChangeLog
>>>     
>> 2008-06-27
>>   
>>> 20:56:26 UTC (rev 106777)
>>> @@ -1,3 +1,7 @@
>>> +2008-06-27  Atsushi Enomoto  <atsushi at ximian.com>
>>> +
>>> +	* PropertyDescriptorCollectionTests.cs : huh.
>>> +
>>>  2008-06-27  Gert Driesen  <drieseng at users.sourceforge.net>
>>>  
>>>  	* EventDescriptorCollectionTests.cs: Enabled tests for Empty, Find
>>> and
>>>
>>> Modified:
>>>
>>>     
>
trunk/mcs/class/System/Test/System.ComponentModel/PropertyDescriptorCollecti
>>   
>>> onTests.cs
>>> ===================================================================
>>> ---
>>>
>>>     
>
trunk/mcs/class/System/Test/System.ComponentModel/PropertyDescriptorCollecti
>>   
>>> onTests.cs	2008-06-27 20:45:49 UTC (rev 106776)
>>> +++
>>>
>>>     
>
trunk/mcs/class/System/Test/System.ComponentModel/PropertyDescriptorCollecti
>>   
>>> onTests.cs	2008-06-27 20:56:26 UTC (rev 106777)
>>> @@ -92,7 +92,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#A2");
>>>  				Assert.IsNull (ex.InnerException, "#A3");
>>>  				Assert.IsNotNull (ex.Message, "#A4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#A5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#A5");
>>>  			}
>>>  
>>>  			try {
>>> @@ -102,7 +102,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#B2");
>>>  				Assert.IsNull (ex.InnerException, "#B3");
>>>  				Assert.IsNotNull (ex.Message, "#B4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#B5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#B5");
>>>  			}
>>>  
>>>  			descriptors = PropertyDescriptorCollection.Empty;
>>> @@ -114,7 +114,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#C2");
>>>  				Assert.IsNull (ex.InnerException, "#C3");
>>>  				Assert.IsNotNull (ex.Message, "#C4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#C5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#C5");
>>>  			}
>>>  
>>>  			try {
>>> @@ -124,7 +124,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#D2");
>>>  				Assert.IsNull (ex.InnerException, "#D3");
>>>  				Assert.IsNotNull (ex.Message, "#D4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#D5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#D5");
>>>  			}
>>>  		}
>>>  
>>> @@ -235,7 +235,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#A2");
>>>  				Assert.IsNull (ex.InnerException, "#A3");
>>>  				Assert.IsNotNull (ex.Message, "#A4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#A5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#A5");
>>>  			}
>>>  
>>>  			descriptors = PropertyDescriptorCollection.Empty;
>>> @@ -247,7 +247,7 @@
>>>  				Assert.AreEqual (typeof
>>> (ArgumentNullException), ex.GetType (), "#B2");
>>>  				Assert.IsNull (ex.InnerException, "#B3");
>>>  				Assert.IsNotNull (ex.Message, "#B4");
>>> -				Assert.AreEqual ("key", ex.ParamName,
>>> "#B5");
>>> +				Assert.AreEqual ("name", ex.ParamName,
>>> "#B5");
>>>  			}
>>>  		}
>>>  
>>>
>>> _______________________________________________
>>> Mono-patches maillist  -  Mono-patches at lists.ximian.com
>>> http://lists.ximian.com/mailman/listinfo/mono-patches
>>>
>>>
>>>     
>>
>>
>>   
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>

_______________________________________________
Mono-devel-list mailing list
Mono-devel-list at lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list



More information about the Mono-devel-list mailing list