Looks good. About organization, I gave F2F feedback. Small comments about the 
nitty gritty...

In assembly/access/dependencies1, can A.dll and B.dll also be generated using 
some overloaded version of csc which takes an assembly name? Not a big deal for 
now since there are only two assemblies checked in, but if you are going to 
need more, you might as well use the csc infrastructure which you are building 
up anyway.

There is Tests\interop\assembly.cs as well as Tests\interop\assembly folder. 
Would be nice if the former was called Fixtures.cs or something like that to 
disambiguate the two.

Could you add a comment to the file saying it is generated by a script, or name 
it as foo.Generated.cs so it obvious that you should not edit it by hand?

Thanks,
Shri


-----Original Message-----
From: Jim Deville 
Sent: Friday, March 06, 2009 11:13 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core@rubyforge.org
Subject: Code Review: interop3

  tfpt review "/shelveset:interop3;REDMOND\jdeville"
  Comment  : 
  * Rearrange .NET tests according to the new test structure
  * Add the csc method to inline C# fixtures into the Ruby code
  ** Should csc.bat be in a path'd scripts directory? Or is it okay having to 
specifiy a path to it?
  * Modify both dev.bats to add 
Merlin\External\Languages\IronRuby\mspec\mspec\bin to %PATH% 
  * Modify IronRuby's dev.bat to setup ~/.mspecrc if ~/.mspecrc doesn't exist.
  ** Should this modification be in both? If so, why not have one Dev.bat that 
conditionally loads internal alias's?
  * Make the default.mspec, which becomes ~/.mspecrc, simply load our default 
configuration file from Merlin\External\Languages\IronRuby\mspec\default.mspec
  


_______________________________________________
Ironruby-core mailing list
Ironruby-core@rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core

Reply via email to