Hey Petit, I just review your patch and I think that you can improve it before get it on SVN. Also I think that is better if you create a bug report in bugzilla an attach your patch, in description you can use "[Patch]" to identify that it is a patch so we can track it in bugzilla like all other patches.
You can see some examples about how we track patches in https://bugzilla.novell.com/show_bug.cgi?id=325809 or you can see also other bugs marked as "patch". So, now let us go to my suggestions to your patch: * Indent your code using Tab instead of spaces like the rest of MWF code and we have an "space" before every "(" in method declaration. * On top of mcs svn module we have CodingStyle file, you can read it to see more about Mono code style. Also you can read it on our wiki: http://www.mono-project.com/Coding_Guidelines. * The method mwfFileView_ColumnClick could be called OnColumnClickFileView to be in accord with all other mwfFileView methods. Also, remove the System.Windows.Forms. from ColumnClickEventHandler, look to the line before and do the same. * Rename ListViewItemComparer to MwfFileViewItemComparer so later if we need to implement another comparer class we will not have problems with names. * In mwfFileView_ColumnClick now ColumnClickEventHandler we have one try-catch that send a error message to console, I dont think that we need this try-catch but if we really need it will be better to prevent errors on MwfFileViewItemComparer class. * AscDesc var is also not a good name for a private member (Coding_Guidelines) maybe you can use something like "mwffileview_order" or "fileview_order". * According to MSDN, after use ListViewItemSorter you dont need to call sort maybe MSDN is wrong (it happens a lot) so is better to test if we really need to call Sort method. * Check also private member names in MwfFileViewItemComparer class. Ah, make sure that the patch is also generated using last Mono SVN trunk. Thank you very much, Everaldo. _______________________________________________ Mono-list maillist - [email protected] http://lists.ximian.com/mailman/listinfo/mono-list
