[Mono-dev] revert of r104029/30/31

Mon May 26 12:14:10 EDT 2008

IMO the parameter names are just significant, not important. I mean,
it's like "significant space" in XML document. If they are regarded
as important, I strongly suggest to not report them. They cost us
lots of hours or days to fix them and hence practically block the
actual API fixes that are actually "important".

Personally I dislike that corcompare change. It could have been
optional to work only for compatibility enthusiasts.

Atsushi Eno

Sebastien Pouliot wrote:
> Hello Gert,
> 
> On Mon, 2008-05-26 at 14:49 +0200, Gert Driesen wrote:
>> Hey Sebastien,
>>
>> For me, API compatibility with MS is important and as such I want to reduce 
>> the number of issues reported by our class status pages to zero (or as close 
>> as possible). Having several hundred issues reported because of mismatches 
>> in argument names can cause us to miss more important mismatches. I hope 
>> that makes it clear as to you why I want to get these argument name changes 
>> in.
> 
> I'm in *total* agreement about fixing parameters names (even if I have
> yet to see a bug report about it) and all other issues pointed by the
> class status page (if it's not important enough to fix then it should
> not be displayed on those pages).
> 
>> One mistake I made is that - while fixing the argument names - I noticed a 
>> misture of spaces and tabs for indentation (and loads of trailing tabs), and 
>> decided to fix this. We all agree now that I could've split these changes. 
>> No problem.
>>
>> A second mistake I made is that I forgot to run the unit tests after my 
>> changes. I don't think you have to convince me of the importance of unit 
>> tests, so we again agree on this.
> 
> /me nods
> 
> However the unit test failure was "pure luck" (well from my point of
> view ;-). For that matter I did a quick review of the patches (from the
> mailing-list archive) and I did not spot it (and I prefer blaming the
> size of the patch that my eyesight ;-).
> 
>> Proposal:
>>
>> If I have some time and you can make time to review it, I'll submit a patch 
>> for the argument name changes for review.
>>
>> Agree?
> 
> Yes. Thanks!
> Sebastien
> 
>> Gert
>>
>> --------------------------------------------------
>> From: "Sebastien Pouliot" <sebastien at ximian.com>
>> Sent: Monday, May 26, 2008 2:40 PM
>> To: "Gert Driesen" <gert.driesen at telenet.be>
>> Cc: "mono-devel-list" <mono-devel-list at lists.ximian.com>
>> Subject: Re: [Mono-dev] revert of r104029/30/31
>>
>>> Gert,
>>>
>>> On Mon, 2008-05-26 at 11:10 +0200, Gert Driesen wrote:
>>>> --------------------------------------------------
>>>> From: "Sebastien Pouliot" <sebastien at ximian.com>
>>>> Sent: Monday, May 26, 2008 1:21 AM
>>>> To: <gert.driesen at pandora.be>
>>>> Cc: "mono-devel-list" <mono-devel-list at lists.ximian.com>
>>>> Subject: [Mono-dev] revert of r104029/30/31
>>>>
>>>>> Gert,
>>>>>
>>>>> Please *post* your patches for review *before* committing them!
>>>>> Those changes were obviously untested since they broke the unit tests.
>>>> Sorry about that. I was too confident that these changes were harmless, 
>>>> my
>>>> mistake.
>>>>
>>>> I'll look into this, and - if you want - post an updated patch for 
>>>> review.
>>> I'm not interested in reviewing a HUGE patch that mix the functional and
>>> non-functional changes together. It's huge, hard to read, easy to
>>> miss(*) and I already have spent too much time on this one (trying to
>>> post-commit review it, reverting it, writing about it...)
>>>
>>> (*) the fact that unit test caught one error does not mean there are no
>>> others - and I don't believe the changes are worth regression(s).
>>>
>>>>> They also mix formatting changes (not all in respect of mono style) 
>>>>> with
>>>>> code changes (parameter names) making this harder, than it has to be, 
>>>>> to
>>>>> review.
>>>> I'm pretty sure all the formatting changes respect the "mono style", but
>>>> feel free to correct me.
>>>>
>>>> I know we had a "discussion" on whether braces for the class/namespace
>>>> should go on a separate line or not, and I know you thought they should 
>>>> go
>>>> on the same line. However, both the mono guidelines
>>> Please show me where in
>>> http://www.mono-project.com/Coding_Guidelines
>>> since all examples keep them on the same line ? (and this is not a code
>>> block either ;-)
>>>
>>> So it's one of the case that's unclear ? just like some other stuff that
>>> was changed (or removed) since the original style guide ?
>>>
>>> I don't mind if, in doubt, you follow your own interpretation. However
>>> please DO NOT start modifying existing code based on them (own
>>> interpretation).
>>>
>>> Fact is that the *style* is there to help everyone, just like doing
>>> *small* patches are. Yes some old SD code breaks a *lot* of rules and I
>>> slowly fixed some when the code needed fixing.
>>>
>>> Why ? because breaking the "as small as possible commits" rule makes
>>> merging harder and SVN history less useful. Updating the current code
>>> style is not gonna offset the cost of this (even less if it requires a
>>> full review because it includes other changes as well).
>>>
>>>> and several committers
>>>> on #mono confirmed that they should go on a separate line.
>>> "confirmed" ? how ? who ? Maybe they just tell you what they do, or what
>>> they like ?
>>>
>>> Seriously we have a document for this. It's not perfect (and feel free
>>> to start _another_ discussion about this) and there are a lot of rules I
>>> didn't like (but I learned to follow them).
>>>
>>> But like I said in doubt just *dont* touch it.
>>>
>>>> I don't think I want to spend an hour or two splitting up the argument 
>>>> name
>>>> changes with the formatting changes,
>>> I understand this because I don't want to spend an hour or two to find
>>> other problems in the diff (and it's clear it's something that needs to
>>> be done).
>>>
>>>> so if you prefer to keep the sources in
>>>> the state they were before my changes then I'll just revert my local
>>>> changes.
>>>>
>>>> My formatting changes were limited to:
>>>>
>>>> * changes loads of spaces to tabs
>>>> * removing loads of trailing tabs
>>>> * moving braces on class/namespace declaration to separate line (probably
>>>> less than 2% of my changes)
>>> Even if 100% of the changes are ok and were made without other changes
>>> (parameters) I still think (and would have told you) that it was not
>>> worth the time you invested in this.
>>>
>>>>> Also note that such *big* changes always cause a lot of conflicts for
>>>>> the code maintainer(s), since they often have uncommitted code or patch
>>>>> in testing. So a bit of warning before committing is a *minimum*.
>>>> True.
>>> Yes, a lot of time could have been saved with a quick email (or ping on
>>> IRC).
>>>
>>> Sebastien
>>>
>>> _______________________________________________
>>> 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
> 