tfpt review "/shelveset:RandomRubyFixes01;REDMOND\curth" Comment : Fix many bugs in Marshal; now passes all but 5 tests Implement Time._load and Time._dump to support marshaling of Time objects Implement "arity" function for method groups Correctly scope the constant when looking for the YAML module in yaml_as -- Curt Hagenlocher curth at microsoft.com -------------- next part -------------- A non-text attachment was scrubbed... Name: RandomRubyFixes01.diff Type: application/octet-stream Size: 80378 bytes Desc: RandomRubyFixes01.diff URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080920/cb7511eb/attachment-0001.obj>
I wonder why you removed spec comments from TimeOps? Not that I object that, just curious if our convention is not to include such things into the code. Also you marked some methods in Marshal as private, while a bunch of them still left with no access modifier, I think we should be more consistent here and mark everything with explicit access modifiers. Otherwise looks great. -- Oleg -----Original Message----- From: Curt Hagenlocher Sent: Saturday, September 20, 2008 7:11 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Code Review: RandomRubyFixes01 tfpt review "/shelveset:RandomRubyFixes01;REDMOND\curth" Comment : Fix many bugs in Marshal; now passes all but 5 tests Implement Time._load and Time._dump to support marshaling of Time objects Implement "arity" function for method groups Correctly scope the constant when looking for the YAML module in yaml_as -- Curt Hagenlocher curth at microsoft.com
Right, our convention is not to copy documentation from ri. I agree on access modifiers. Tomas -----Original Message----- From: Oleg Tkachenko Sent: Sunday, September 21, 2008 3:26 PM To: Curt Hagenlocher; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Code Review: RandomRubyFixes01 I wonder why you removed spec comments from TimeOps? Not that I object that, just curious if our convention is not to include such things into the code. Also you marked some methods in Marshal as private, while a bunch of them still left with no access modifier, I think we should be more consistent here and mark everything with explicit access modifiers. Otherwise looks great. -- Oleg -----Original Message----- From: Curt Hagenlocher Sent: Saturday, September 20, 2008 7:11 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Code Review: RandomRubyFixes01 tfpt review "/shelveset:RandomRubyFixes01;REDMOND\curth" Comment : Fix many bugs in Marshal; now passes all but 5 tests Implement Time._load and Time._dump to support marshaling of Time objects Implement "arity" function for method groups Correctly scope the constant when looking for the YAML module in yaml_as -- Curt Hagenlocher curth at microsoft.com
I thought I had gotten all the access modifiers but I think I got bored with adding them and lost focus. ;) I''ll fix that. -----Original Message----- From: Tomas Matousek Sent: Sunday, September 21, 2008 3:34 PM To: Oleg Tkachenko; Curt Hagenlocher; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Code Review: RandomRubyFixes01 Right, our convention is not to copy documentation from ri. I agree on access modifiers. Tomas -----Original Message----- From: Oleg Tkachenko Sent: Sunday, September 21, 2008 3:26 PM To: Curt Hagenlocher; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Code Review: RandomRubyFixes01 I wonder why you removed spec comments from TimeOps? Not that I object that, just curious if our convention is not to include such things into the code. Also you marked some methods in Marshal as private, while a bunch of them still left with no access modifier, I think we should be more consistent here and mark everything with explicit access modifiers. Otherwise looks great. -- Oleg -----Original Message----- From: Curt Hagenlocher Sent: Saturday, September 20, 2008 7:11 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Code Review: RandomRubyFixes01 tfpt review "/shelveset:RandomRubyFixes01;REDMOND\curth" Comment : Fix many bugs in Marshal; now passes all but 5 tests Implement Time._load and Time._dump to support marshaling of Time objects Implement "arity" function for method groups Correctly scope the constant when looking for the YAML module in yaml_as -- Curt Hagenlocher curth at microsoft.com
Does that mean I should remove the documentation from the Numeric classes, too? In that code I had put them in as C# xml documentation i.e. ///<summary>... kind of stuff. Is this not wanted any more? Pete -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Tomas Matousek Sent: Sunday,21 September 21, 2008 23:34 To: Oleg Tkachenko; Curt Hagenlocher; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] Code Review: RandomRubyFixes01 Right, our convention is not to copy documentation from ri. I agree on access modifiers. Tomas