Hello,
Yes the code works. I've done geninit, recompiled and tested using a script I wrote. I'll work on the other issues you mentioned and resubmit my changes for review. Also I've already signed the agreement with ssiadmin. Thanks for your comments Pascal _____ From: Shri Borde [mailto:shri.bo...@microsoft.com] Sent: August-06-09 4:28 PM To: pascal.norman...@convergentware.com; ironruby-core@rubyforge.org Subject: RE: [Ironruby-core] Code Review: Thread priority Thanks for the first commit, Pascal! I have a few comments. I don't think this code is actually going to work. You need to call "geninit" as mentioned at http://wiki.github.com/ironruby/ironruby/modifying-the-sources. Ideally, all fixes should have specs. This would have easily caught the issue above. For Thread#priority, there are no existing specs unfortunately. Could you please add specs to Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\rubyspec\core\thread \priority_spec.rb to atleast ensure that the attribute can be read and set? Also, should the "priority" argument be tagged with DefaultProtocol? You can add specs modeling the corner cases in the "Corner cases and negative scenarios" section at http://rubyspec.org/wiki/rubyspec/Style_Guide, which is linked from http://wiki.github.com/ironruby/ironruby/modifying-the-sources which also briefly describes DefaultProtocol. Another corner case is a dead thread. http://msdn.microsoft.com/en-us/library/system.threading.thread.priority.asp x says that an ThreadStateException will be thrown. Not sure what MRI does. The return type of the getter method should also be int, not object. Both will work, but its preferable to strongly type the code as much as possible. Did you run irtests.bat with 0 failures? Also, have you sent email to ssiadmin as mentioned at http://wiki.github.com/ironruby/ironruby/contributing? Thanks again, and looking forward to seeing the specs and pulling this in. From: ironruby-core-boun...@rubyforge.org [mailto:ironruby-core-boun...@rubyforge.org] On Behalf Of Pascal Normandin Sent: Tuesday, August 04, 2009 6:13 AM To: ironruby-core@rubyforge.org Subject: [Ironruby-core] Code Review: Thread priority Modification to be able to set and read the priority of a thread. Main commit: http://github.com/PascalN2/ironruby/commit/3a0373379b79dbe7fff603ad61ab726d7 de7f305 Small modification for coding standard: http://github.com/PascalN2/ironruby/commit/47f0de87e908981f553732e97f948a33b 164f202 Please let me know how that looks. Thanks; Pascal Normandin
_______________________________________________ Ironruby-core mailing list Ironruby-core@rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core
