Shri Borde
2009-Apr-02 19:08 UTC
[Ironruby-core] FYI review - Jirapong''s fixes for GzipFile#finish
FYI review only... This is Jirapong''s change. A big thanks to Jirapong for his first contribution to IronRuby! Looking forward to many more! I have pushed it to my fork. The individual commits are shown below, but there will be a bit of noise as Jirapong bounced questions off me. I should probably have squashed his individual commits into one to remove the noise. Might do that for the next round... http://github.com/shri/ironruby/commit/62b0d91e3fdc6c557ed7f698e1026524ee8f7af6 http://github.com/shri/ironruby/commit/88a3902ffa3eb2f6fb145142f144796e7a021602 http://github.com/shri/ironruby/commit/24b069c0a8f1448977dd0f6b1f7ae7f7b556ab18 http://github.com/Jirapong/ironruby/commit/2926dd7452a9e09e269d743a9641131c5a9278bd http://github.com/shri/ironruby/commit/ec7270c2934c81a88dcc4ce274db6404ec2c7153 - My code review http://github.com/shri/ironruby/commit/c71e026eca1134d77a87bd792b1d93c60e32bf81 GitDiff.bat c9a26adb7c443afc192514c72806e95b178d5088 c71e026eca1134d77a87bd792b1d93c60e32bf81 GzipFile#finish should close the GzipReader/GzipWriter, but not the underlying IO object. Thanks, Shri -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20090402/e8deb70b/attachment-0001.html>
Jim Deville
2009-Apr-02 20:20 UTC
[Ironruby-core] FYI review - Jirapong''s fixes for GzipFile#finish
The debugger method shouldn''t be in spec_helper.rb. It''s a IronRuby specific method and that file is a MSpec file. Also, it''s missing an end. Other than that, test looks good. JD From: Shri Borde Sent: Thursday, April 02, 2009 12:08 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: FYI review - Jirapong''s fixes for GzipFile#finish FYI review only... This is Jirapong''s change. A big thanks to Jirapong for his first contribution to IronRuby! Looking forward to many more! I have pushed it to my fork. The individual commits are shown below, but there will be a bit of noise as Jirapong bounced questions off me. I should probably have squashed his individual commits into one to remove the noise. Might do that for the next round... http://github.com/shri/ironruby/commit/62b0d91e3fdc6c557ed7f698e1026524ee8f7af6 http://github.com/shri/ironruby/commit/88a3902ffa3eb2f6fb145142f144796e7a021602 http://github.com/shri/ironruby/commit/24b069c0a8f1448977dd0f6b1f7ae7f7b556ab18 http://github.com/Jirapong/ironruby/commit/2926dd7452a9e09e269d743a9641131c5a9278bd http://github.com/shri/ironruby/commit/ec7270c2934c81a88dcc4ce274db6404ec2c7153 - My code review http://github.com/shri/ironruby/commit/c71e026eca1134d77a87bd792b1d93c60e32bf81 GitDiff.bat c9a26adb7c443afc192514c72806e95b178d5088 c71e026eca1134d77a87bd792b1d93c60e32bf81 GzipFile#finish should close the GzipReader/GzipWriter, but not the underlying IO object. Thanks, Shri -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20090402/820511c2/attachment.html>