[Ironruby-core] Code Review: YamlTestsAndStream

Michael Letterle michael.letterle at gmail.com
Fri Jun 13 21:14:43 EDT 2008


I was waiting to see if this was fixed in the latest drop... but the build
output for the YAML library seems a little... wonky compared to the rest of
the projects.  Seems like it should be the same as IronRuby.Libraries. Bug?

On Fri, Jun 13, 2008 at 8:58 PM, Tomas Matousek <
Tomas.Matousek at microsoft.com> wrote:

>  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 at 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 at rubyforge.org
> http://rubyforge.org/mailman/listinfo/ironruby-core
>
>


-- 
Michael Letterle
[Polymath Prokrammer]
http://blog.prokrams.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080613/e828e48b/attachment.html>


More information about the Ironruby-core mailing list