[Mono-dev] Mono Winforms on Mac

[Carlos Alberto Cortez]

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>
>> >> >
>> >>
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20101030/2eddb8e7/attachment.html