http://github.com/shri/ironruby/commit/a5d2d23b5c97546c478189d20b4d2c1b240e7159
Tomas has reviewed this.
Added support to YAML.quick_emit to be called on nested objects where the
same emitter needs to be flowed through
Array.pack("a") was not dealing with nil input
yaml.rb should to "require stringio" like MRI does
Deleted a second copy of yaml.rb from the IronRuby.Libraries.Yaml folder
Added MutableString.Dump property to be able to inspect large strings in
VS.
ir.exe -X:ExceptionDetail was not working in interactive mode because
ir.exe catches the exception and does not call
RubyExceptionData.SetCompiledTrace like a rescue block in Ruby code would
do. Fixed this
Thanks,
Shri
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20090313/ed04e8a5/attachment.html>
This is a review on both this and the Gzip commit:
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\fixtures\class.rb
o Please use parenthesis for declaring methods for consistency
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\each_node_spec.rb
o Please use += instead of i = i + 1
o Have we checked that not being a subclass of YAML::Syck::Map is ok? I
suspect it is an implementation detail, but I want to check. If it is
implementation detail, then we probably don''t need to spec that one.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\emitter_spec.rb
o Don''t spec method lists. For one, a implementation may choose a
superset (so == is wrong), for two, RubySpec doesn''t spec method lists.
o Same question about the subclass, which makes me wonder if that 1st spec is
needed.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\quick_emit_spec.rb
o Subclass spec
o Don''t spec exception messages, only exception types.
o I would add a ScratchPad.clear to a before(:each)
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\to_yaml_spec.rb
o Don''t spec method lists.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\fixtures\common.rb
o Get rid of EmitterMethods and OutMethods
o Line 54 and 68, no need to do != nil. If the object is nil it will be false.
Also, get rid of the "then"s
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\gzipreader\wrap_spec.rb
o You''re using ScratchPad when there is no reason. 22, 29, put your
should in the Zlib blocks.
o 38 is fine
o 56: don''t spec the exception message
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\zlib\gzipwriter\flush_spec.rb
o Don''t spec error messages
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubysync\library\zlib\gzipwriter\wrap_spec.rb
o 28: should in the block
o 34
1. Zlib::GzipWriter.wrap(@io) do |gz|
From: Shri Borde
Sent: Friday, March 13, 2009 3:41 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: FYI review: YAML improvements
http://github.com/shri/ironruby/commit/a5d2d23b5c97546c478189d20b4d2c1b240e7159
Tomas has reviewed this.
Added support to YAML.quick_emit to be called on nested objects where the
same emitter needs to be flowed through
Array.pack("a") was not dealing with nil input
yaml.rb should to "require stringio" like MRI does
Deleted a second copy of yaml.rb from the IronRuby.Libraries.Yaml folder
Added MutableString.Dump property to be able to inspect large strings in
VS.
ir.exe -X:ExceptionDetail was not working in interactive mode because
ir.exe catches the exception and does not call
RubyExceptionData.SetCompiledTrace like a rescue block in Ruby code would
do. Fixed this
Thanks,
Shri
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20090313/71c5a1e8/attachment.html>
Hit ctl-enter instead of shift-enter:
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubysync\library\zlib\gzipwriter\wrap_spec.rb
o 28: should in the block
o 34
1. Zlib::GzipWriter.wrap(@io) do |gz|
:end_of_block
end.should == :end_of_block
o 64: error messages
JD
From: Jim Deville
Sent: Friday, March 13, 2009 11:10 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: FYI review: YAML improvements
This is a review on both this and the Gzip commit:
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\fixtures\class.rb
o Please use parenthesis for declaring methods for consistency
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\each_node_spec.rb
o Please use += instead of i = i + 1
o Have we checked that not being a subclass of YAML::Syck::Map is ok? I
suspect it is an implementation detail, but I want to check. If it is
implementation detail, then we probably don''t need to spec that one.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\emitter_spec.rb
o Don''t spec method lists. For one, a implementation may choose a
superset (so == is wrong), for two, RubySpec doesn''t spec method lists.
o Same question about the subclass, which makes me wonder if that 1st spec is
needed.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\quick_emit_spec.rb
o Subclass spec
o Don''t spec exception messages, only exception types.
o I would add a ScratchPad.clear to a before(:each)
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\to_yaml_spec.rb
o Don''t spec method lists.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\fixtures\common.rb
o Get rid of EmitterMethods and OutMethods
o Line 54 and 68, no need to do != nil. If the object is nil it will be false.
Also, get rid of the "then"s
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\gzipreader\wrap_spec.rb
o You''re using ScratchPad when there is no reason. 22, 29, put your
should in the Zlib blocks.
o 38 is fine
o 56: don''t spec the exception message
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\zlib\gzipwriter\flush_spec.rb
o Don''t spec error messages
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubysync\library\zlib\gzipwriter\wrap_spec.rb
o 28: should in the block
o 34
1. Zlib::GzipWriter.wrap(@io) do |gz|
From: Shri Borde
Sent: Friday, March 13, 2009 3:41 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: FYI review: YAML improvements
http://github.com/shri/ironruby/commit/a5d2d23b5c97546c478189d20b4d2c1b240e7159
Tomas has reviewed this.
Added support to YAML.quick_emit to be called on nested objects where the
same emitter needs to be flowed through
Array.pack("a") was not dealing with nil input
yaml.rb should to "require stringio" like MRI does
Deleted a second copy of yaml.rb from the IronRuby.Libraries.Yaml folder
Added MutableString.Dump property to be able to inspect large strings in
VS.
ir.exe -X:ExceptionDetail was not working in interactive mode because
ir.exe catches the exception and does not call
RubyExceptionData.SetCompiledTrace like a rescue block in Ruby code would
do. Fixed this
Thanks,
Shri
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20090313/08910b2a/attachment-0001.html>
FYI: After talking with Brian from RubySpec, I have pulled the specs that check
for be_kind_of(YAML::Syck::*). We have decided that it is implementation detail,
and since IronRuby and RbYaml don''t support it, it isn''t
something that should be specced.
JD
From: Jim Deville
Sent: Friday, March 13, 2009 11:13 PM
To: Jim Deville; Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: FYI review: YAML improvements
Hit ctl-enter instead of shift-enter:
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubysync\library\zlib\gzipwriter\wrap_spec.rb
o 28: should in the block
o 34
1. Zlib::GzipWriter.wrap(@io) do |gz|
:end_of_block
end.should == :end_of_block
o 64: error messages
JD
From: Jim Deville
Sent: Friday, March 13, 2009 11:10 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: FYI review: YAML improvements
This is a review on both this and the Gzip commit:
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\fixtures\class.rb
o Please use parenthesis for declaring methods for consistency
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\each_node_spec.rb
o Please use += instead of i = i + 1
o Have we checked that not being a subclass of YAML::Syck::Map is ok? I
suspect it is an implementation detail, but I want to check. If it is
implementation detail, then we probably don''t need to spec that one.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\emitter_spec.rb
o Don''t spec method lists. For one, a implementation may choose a
superset (so == is wrong), for two, RubySpec doesn''t spec method lists.
o Same question about the subclass, which makes me wonder if that 1st spec is
needed.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\quick_emit_spec.rb
o Subclass spec
o Don''t spec exception messages, only exception types.
o I would add a ScratchPad.clear to a before(:each)
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\to_yaml_spec.rb
o Don''t spec method lists.
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\yaml\fixtures\common.rb
o Get rid of EmitterMethods and OutMethods
o Line 54 and 68, no need to do != nil. If the object is nil it will be false.
Also, get rid of the "then"s
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\gzipreader\wrap_spec.rb
o You''re using ScratchPad when there is no reason. 22, 29, put your
should in the Zlib blocks.
o 38 is fine
o 56: don''t spec the exception message
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubyspec\library\zlib\gzipwriter\flush_spec.rb
o Don''t spec error messages
*
C:\vsl\rubysync\Merlin\External\Languages\IronRuby\mspec\rubysync\library\zlib\gzipwriter\wrap_spec.rb
o 28: should in the block
o 34
1. Zlib::GzipWriter.wrap(@io) do |gz|
From: Shri Borde
Sent: Friday, March 13, 2009 3:41 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: FYI review: YAML improvements
http://github.com/shri/ironruby/commit/a5d2d23b5c97546c478189d20b4d2c1b240e7159
Tomas has reviewed this.
Added support to YAML.quick_emit to be called on nested objects where the
same emitter needs to be flowed through
Array.pack("a") was not dealing with nil input
yaml.rb should to "require stringio" like MRI does
Deleted a second copy of yaml.rb from the IronRuby.Libraries.Yaml folder
Added MutableString.Dump property to be able to inspect large strings in
VS.
ir.exe -X:ExceptionDetail was not working in interactive mode because
ir.exe catches the exception and does not call
RubyExceptionData.SetCompiledTrace like a rescue block in Ruby code would
do. Fixed this
Thanks,
Shri
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20090315/1dbe0718/attachment-0001.html>