[Mono-dev] Mono Winforms on Mac

[Ralph Leckett]

Thank you Carlos.

Ralph (the open source newbie)


On 10/30/10, Carlos Alberto Cortez <calberto.cortez at gmail.com> wrote:
> Hello,
>
> I will step up, since I have more experience on the code, but based on its
> size, I may be giving feedback piece by piece.
>
> Carlos.
>
> 2010/10/30 Miguel de Icaza <miguel at novell.com>
>
>> Hello Ralph,
>>
>> Does the Mono Project have it's own format utility? It would be nice
>>> to make all the code consistent no matter what that format is.
>>>
>>
>> We do not.   And I do agree with you that some of the practices on
>> Windows.Forms were less than ideal, reading your patch made me realize
>> that
>> the Windows.Forms team created their own rules that did not really match
>> the
>> Mono guidelines and that this needs to be fixed.
>>
>> I am willing to change the formatting on the Windows.Forms codebase in
>> about half the cases that your code did, but this is secondary to getting
>> your patches reviewed and merged.
>>
>> In general, in open source, there is an unspoken rule that you always
>> adapt
>> your code to the upstream coding style.   This means that reviewing
>> patches
>> can focus on the actual substance of the problem instead of having to
>> chase
>> down the changes line-by-line.   The patch that I posted to the mailing
>> list
>> contains the "cleaned up" version of your patch after I removed all
>> the superfluous changes that were still on it.
>>
>> But we are left with a jumbo patch with large changes and no explanation
>> of
>> what the change does, why it does it.   Ideally, you could resubmit the
>> patches on a feature-by-feature basis, this would accelerate the review.
>> As you noticed by the lack of participation on the thread, nobody really
>> has
>> stepped up to review the jumbo patch after I cleaned it up.   The changes
>> are just too big as they are now.
>>
>> If nobody steps up, I will add it to my own list of tasks to do, but right
>> now I am tied up for the next two weeks.
>>
>> Miguel
>>
>>>
>>> On 10/30/10, Miguel de Icaza <miguel at novell.com> wrote:
>>> > Hello Ralph,
>>> >
>>> >     Have you given any consideration to providing a roadmap to your
>>> changes
>>> > so that they can be individually reviewed?
>>> >
>>> >     Since you reformatted the source code, it is not possible to apply
>>> the
>>> > patches directly, we still have to apply every change by hand and it
>>> would
>>> > be useful to have the different patches and changes reviewed
>>> independently.
>>> >
>>> > Miguel
>>> >
>>> > On Sat, Oct 30, 2010 at 10:34 AM, Ralph Leckett <rleckett at gmail.com>
>>> wrote:
>>> >
>>> >> Attached is another set of updates for Mac Mono Winforms. My .net
>>> >> applications now work quite nicely with my changes to Mono.
>>> >>
>>> >> NativeWindow has been simplified to using Hwnd to provide the
>>> >> Handle-Window lookup. I have found that storing handles in Hashtable
>>> >> sometimes results in the handle being lost with the result being
>>> >> zombie windows. Replacing Hashtable with a linked list solved the
>>> >> problem. Placing the most recently used link at the head of the linked
>>> >> list maintains efficiency.
>>> >>
>>> >> MS Windows places paint events at the lowest priority with all other
>>> >> events preempting WM_PAINT. A separate queue (linked list in my case)
>>> >> for paint events is required for Mono to duplicate this effect.
>>> >>
>>> >> I find the exposure of method variables to outside objects without
>>> >> using properties to be one of the poorer coding practices used in
>>> >> Mono. Apart from that, I found Mono to be quite well written and a
>>> >> pleasure to work on.
>>> >>
>>> >> Ralph
>>> >>
>>> >> On 10/20/10, Ralph Leckett <rleckett at gmail.com> wrote:
>>> >> > The namespaces in the code caused compile errors in one of my test
>>> >> > environments so I had to remove them.
>>> >> >
>>> >> > Attached is an update to my updates to fix this problem:
>>> >> > https://bugzilla.novell.com/show_bug.cgi?id=438281
>>> >> >
>>> >> > Ralph
>>> >> >
>>> >> > On 10/19/10, Miguel de Icaza <miguel at novell.com> wrote:
>>> >> >> Hello,
>>> >> >>
>>> >> >>     I have created a diff file from the formatted sources that
>>> removes
>>> >> >> some of the other automatic changes that probably came out of some
>>> >> >> refactoring tool.   The patch itself can not be used directly
>>> against
>>> >> >> Mono's source code since it is still a diff from formatted to
>>> >> >> formatted,
>>> >> >> not from Mono to formatted.    This means that we are going to have
>>> to
>>> >> >> apply every patch by hand.
>>> >> >>
>>> >> >>     I personally like the changes that were made by the tool, like
>>> >> >> dropping the gratuitous overuse of namespaces in the
>>> Managed.Winforms
>>> >> >> code, and would love to plug those changes back in the future.
>>> >> >>
>>> >> >>     In terms of how to get the changes merged, ideally, Ralph could
>>> >> >> review the attached file and provide a roadmap of what each of the
>>> >> >> changes do.   There are some that I suspect wont be controversial,
>>> for
>>> >> >> example:
>>> >> >>
>>> >> >>      * There is a refactoring of the Clipboard code, this seems
>>> >> >>        straightforward, but it still needs a review.
>>> >> >>
>>> >> >>      * Some look like simple bug fixes, but it would be good to
>>> >> >>        know what were they fixing (for example the ListView.cs
>>> >> >>        changes).
>>> >> >>
>>> >> >>     There are other pieces that look like they went through a lot
>>> >> >> of
>>> >> >> work, and a more detailed explanation of the changes would be
>>> useful,
>>> >> >> in
>>> >> >> this group, I would include:
>>> >> >>
>>> >> >>      * Hwnd.cs
>>> >> >>
>>> >> >>      * XplatUICarbon.cs
>>> >> >>
>>> >> >>     There are some that probably we can skip, like the
>>> System.Drawing
>>> >> >> tracing facility.
>>> >> >>
>>> >> >>      The cleaned up patch is here:
>>> >> >>
>>> >> >>      http://tirania.org/tmp/changes.gz
>>> >> >>
>>> >> >>      This is the diffstat.  aNot sure how to deal with this,
>>> >> >> perhaps
>>> >> >> with a Google doc, where we can use colors to flag what is applied
>>> and
>>> >> >> what not?   Or perhaps we remove parts of the patch when we have
>>> >> >> manually applied that bit?
>>> >> >>
>>> >> > <snip>
>>> >> >
>>> >>
>>> >
>>>
>>
>>
>