Ah, in that case, you do need to include the new Initializers.Generated.cs in 
the commit. This does not automatically get generated during a build, and so 
needs to be in checked in to GIT.

From: ironruby-core-boun...@rubyforge.org 
[mailto:ironruby-core-boun...@rubyforge.org] On Behalf Of Pascal Normandin
Sent: Thursday, August 06, 2009 2:21 PM
To: ironruby-core@rubyforge.org
Subject: Re: [Ironruby-core] Code Review: Thread priority

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.aspx 
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/3a0373379b79dbe7fff603ad61ab726d7de7f305

Small modification for coding standard:
http://github.com/PascalN2/ironruby/commit/47f0de87e908981f553732e97f948a33b164f202

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

Reply via email to