tfpt review "/shelveset:crit_argf;REDMOND\jdeville" Comment : cleans up a few critical tags: * re-patches mock.rb with the patch to remove respond_to last * implements a basic implementation of ARGF, there are many holes that still need to be filled here, but the basic idea works, and the specs are no longer critical. -------------- next part -------------- A non-text attachment was scrubbed... Name: crit_argf.diff Type: application/octet-stream Size: 33313 bytes Desc: crit_argf.diff URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100121/9b4dbe4f/attachment-0001.obj>
Could you add comments "TODO: thread safety" to the added fields.
There are several places that access them in thread unsafe way.
Could you also replace
                    return
MutableString.CreateAscii(CommandLineArguments[_currentFileIndex].ToString());
with
 	    // TODO: convert any non-string
                    return
(MutableString)CommandLineArguments[_currentFileIndex];
?
 Tomas
-----Original Message-----
From: Jim Deville 
Sent: Wednesday, January 20, 2010 5:06 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: crit_argf
  tfpt review "/shelveset:crit_argf;REDMOND\jdeville"
  Comment  : 
  cleans up a few critical tags:
  * re-patches mock.rb with the patch to remove respond_to last
  * implements a basic implementation of ARGF, there are many holes that still
need to be filled here, but the basic idea works, and the specs are no longer
critical.
Will do.
-----Original Message-----
From: Tomas Matousek 
Sent: Wednesday, January 20, 2010 5:53 PM
To: Jim Deville; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: crit_argf
Could you add comments "TODO: thread safety" to the added fields.
There are several places that access them in thread unsafe way.
Could you also replace
                    return
MutableString.CreateAscii(CommandLineArguments[_currentFileIndex].ToString());
with
 	    // TODO: convert any non-string
                    return
(MutableString)CommandLineArguments[_currentFileIndex];
?
 Tomas
-----Original Message-----
From: Jim Deville 
Sent: Wednesday, January 20, 2010 5:06 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: crit_argf
  tfpt review "/shelveset:crit_argf;REDMOND\jdeville"
  Comment  : 
  cleans up a few critical tags:
  * re-patches mock.rb with the patch to remove respond_to last
  * implements a basic implementation of ARGF, there are many holes that still
need to be filled here, but the basic idea works, and the specs are no longer
critical.