1) The lookup for Stream constant is slightly different in Ruby:
require 'YAML' class Module def const_missing name puts "missing #{name}" $S end end module YAML $S = Stream remove_const "Stream" end p YAML::load_stream(File.new("C:\\temp\\yaml.txt")) --- To achieve the same behavior use RubyUtils.GetConstant(context, self, "Stream", false) instead of TryLookupConstant. This method also throws the right exception on failure. 2) In YAML.Stream.Emit: return RubyYaml.DumpAll(context, RubyUtils.GetExecutionContext(context).GetModule(typeof(YamlStreamOps)), self.Documents, io); Since DumpAll method doesn't use "self" parameter, it's unnecessary to create one. It would be better to define a new method that doesn't take the argument and call it from Emit. 3) YamlStream fields seem to be readonly, yet they are not marked as such. Also, it seems that you expect _documents to be non-nullable (mark it by /*!*/ if so). 4) YAML::Stream could be used as a base class in Ruby: class MyStream < YAML::Stream end To enable that, YamlStream class needs to have a default constructor (could be protected). Tomas -----Original Message----- From: Oleg Tkachenko Sent: Friday, June 13, 2008 5:09 PM To: IronRuby External Code Reviewers Cc: ironruby-core@rubyforge.org Subject: Code Review: YamlTestsAndStream tfpt review "/shelveset:YamlTestsAndStream;REDMOND\olegtkac" Comment : Adds some basic unit tests (taken from yaml spec and ruby 1.8.6). Fixes a bug with registration of predefined ruby contructors, which allows to load more that one yaml document. Checks if YAML::Stream class is avaliable and fails gracefully if not. Introduces YAML::Stream class implementation. -- Oleg
_______________________________________________ Ironruby-core mailing list Ironruby-core@rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core