Code Review: interop3

Ruby IronRuby
1

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
2

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

3

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
4

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.