Forum: IronRuby Code Review: interop3

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Ade8632553a9243ae05fc920f68644c1?d=identicon&s=25 Jim Deville (Guest)
on 2009-03-07 08:14
(Received via mailing list)
Attachment: interop3.diff (70 KB)
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
Aea6cfe04952626ab630bde47ff82f89?d=identicon&s=25 Shri Borde (Guest)
on 2009-03-10 01:08
(Received via mailing list)
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
Ade8632553a9243ae05fc920f68644c1?d=identicon&s=25 Jim Deville (Guest)
on 2009-03-11 22:00
(Received via mailing list)
Attachment: interop4.diff (70 KB)
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
Aea6cfe04952626ab630bde47ff82f89?d=identicon&s=25 Shri Borde (Guest)
on 2009-03-11 22:18
(Received via mailing list)
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.
This topic is locked and can not be replied to.