[Ironruby-core] Code Review: interop3

Shri Borde Shri.Borde at microsoft.com
Wed Mar 11 17:14:06 EDT 2009


Yup, you should update both copies of dev.bat since we do want mspec easily (and similarly) usable with TFS as well. You could combine the two copies if you want. You will have to conditionalize other things like setting of RUBY18_EXE etc as well. You can diff the two copies to see everything that would need to be conditionalized. When I cloned it, it was simpler to clone it since it's a small file anyway, and my thinking was that I can go back and refactor it if needed once things settles down. 

-----Original Message-----
From: Jim Deville 
Sent: Wednesday, March 11, 2009 1:50 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: interop3 

FYI review:
Modified csc.rb to generate arbitrary assemblies and renamed the main generated code to fixtures.generated.cs. 

JD
  tfpt review "/shelveset:interop4;REDMOND\jdeville"
  Comment  : 
  * Rearrange .NET tests according to the new test structure
  * Add the csc and assembly methods to inline C# fixtures into the Ruby code
  * 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
  




> -----Original Message-----
> From: Shri Borde
> Sent: Monday, March 09, 2009 5:08 PM
> To: Jim Deville; IronRuby External Code Reviewers
> Cc: ironruby-core at rubyforge.org
> Subject: RE: Code Review: interop3
> 
> 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 at 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
> 
> 



More information about the Ironruby-core mailing list