# Code Review: Update RubySpec **URL:** \ **Category:** IronRuby\ **Created:** [April 22, 2009, 5:18am UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129 "2009-04-22T05:18:34Z")\ **Posts on this page:** 5\ **Page:** 1
**Author:** ![raker\_t](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/r/ecae2f/32.png) [@raker\_t](https://www.ruby-forum.com/u/raker_t)\ **Post date:** [April 22, 2009, 5:18am UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129/1 "2009-04-22T05:18:34Z")
I’m updating RubySpec to the newest revision from Git. Most of this is boring stuff; just updating files and rebuilding tags. However, a couple of these commits are code changes to help the specs run smoother. JD - ``` add ci_files to default.mspec so mspec ci ``` works[http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a](http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a) - ``` add a filtered function to mspec to simplify ``` default.mspec[http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2](http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2) * * * - ``` categorizing uncategorized ``` specs[http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657](http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657) * * * - ``` removing extra spec ``` files[http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6](http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6) - ``` update rubyspec and ``` mspec[http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e](http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e) - ``` add a file with the git sha1 hashes of current mspec and ``` rubyspec[http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73](http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73) - ``` implement File.umask in order to run the ``` specs[http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef](http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef) * * * - ``` Implement Process.waitall (needed by Process ``` specs)[http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e](http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e) * * * - ``` rebuilt ironruby ``` tags[http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475](http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475) The commits marked with \*\*\* are more than just infrastructure work. These are actual code changes that need review. Thanks JD ---
**Author:** ![raker\_t](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/r/ecae2f/32.png) [@raker\_t](https://www.ruby-forum.com/u/raker_t)\ **Post date:** [April 22, 2009, 9:00am UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129/2 "2009-04-22T09:00:50Z")
What is umask supposed to do? Tomas From: Jim D. Sent: Tuesday, April 21, 2009 8:12 PM To: removed\_email\_address@domain.invalid; IronRuby External Code R. Subject: Code Review: Update RubySpec I’m updating RubySpec to the newest revision from Git. Most of this is boring stuff; just updating files and rebuilding tags. However, a couple of these commits are code changes to help the specs run smoother. JD - ``` add ci_files to default.mspec so mspec ci ``` works[http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a](http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a) - ``` add a filtered function to mspec to simplify ``` default.mspec[http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2](http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2) * * * - ``` categorizing uncategorized ``` specs[http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657](http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657) * * * - ``` removing extra spec ``` files[http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6](http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6) - ``` update rubyspec and ``` mspec[http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e](http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e) - ``` add a file with the git sha1 hashes of current mspec and ``` rubyspec[http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73](http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73) - ``` implement File.umask in order to run the ``` specs[http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef](http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef) * * * - ``` Implement Process.waitall (needed by Process ``` specs)[http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e](http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e) * * * - ``` rebuilt ironruby ``` tags[http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475](http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475) The commits marked with \*\*\* are more than just infrastructure work. These are actual code changes that need review. Thanks JD ---
**Author:** ![raker\_t](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/r/ecae2f/32.png) [@raker\_t](https://www.ruby-forum.com/u/raker_t)\ **Post date:** [April 22, 2009, 8:28pm UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129/3 "2009-04-22T20:28:46Z")
Umask is a mask on file creation. By itself it does nothing more than what I’ve implemented (that I can tell), but File.open (and other file creation methods) should look at it to derive the default permissions. The attached script shows the basic idea, and here’s it’s output: [9] \> ruby .\temp.rb Umask is 0 100644 Umask is 200 100444 Umask is 400 100644 Umask is 600 100444 If you run it, it will create 4 files (for the 4 tested modes) so you can see the permissions applied. I looked into adding this support to File.open, but I’m not certain where to begin (I found public RuleGenerator Open(), which I don’t know enough about yet). JD From: Tomas M. Sent: Wednesday, April 22, 2009 12:00 AM To: Jim D.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec What is umask supposed to do? Tomas From: Jim D. Sent: Tuesday, April 21, 2009 8:12 PM To: removed\_email\_address@domain.invalid; IronRuby External Code R. Subject: Code Review: Update RubySpec I’m updating RubySpec to the newest revision from Git. Most of this is boring stuff; just updating files and rebuilding tags. However, a couple of these commits are code changes to help the specs run smoother. JD - ``` add ci_files to default.mspec so mspec ci ``` works[http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a](http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a) - ``` add a filtered function to mspec to simplify ``` default.mspec[http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2](http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2) * * * - ``` categorizing uncategorized ``` specs[http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657](http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657) * * * - ``` removing extra spec ``` files[http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6](http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6) - ``` update rubyspec and ``` mspec[http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e](http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e) - ``` add a file with the git sha1 hashes of current mspec and ``` rubyspec[http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73](http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73) - ``` implement File.umask in order to run the ``` specs[http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef](http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef) * * * - ``` Implement Process.waitall (needed by Process ``` specs)[http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e](http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e) * * * - ``` rebuilt ironruby ``` tags[http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475](http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475) The commits marked with \*\*\* are more than just infrastructure work. These are actual code changes that need review. Thanks JD ---
**Author:** ![raker\_t](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/r/ecae2f/32.png) [@raker\_t](https://www.ruby-forum.com/u/raker_t)\ **Post date:** [April 22, 2009, 11:32pm UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129/4 "2009-04-22T23:32:55Z")
Finished offline and F2F with Tomas. JD From: Tomas M. Sent: Wednesday, April 22, 2009 11:33 AM To: Jim D.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec I see - umask maintains a global state. Any global state maintained by libraries should be stored on RubyContext, not in static fields. Use RubyContext.GetOrCreateLibraryData/TryGetLibraryData (if you search the library you’ll find some use cases). Also defining a struct just to store an integer seems unnecessary (besides it’s not good to define structs with mutable state). Tomas From: Jim D. Sent: Wednesday, April 22, 2009 11:09 AM To: Tomas M.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec Umask is a mask on file creation. By itself it does nothing more than what I’ve implemented (that I can tell), but File.open (and other file creation methods) should look at it to derive the default permissions. The attached script shows the basic idea, and here’s it’s output: [9] \> ruby .\temp.rb Umask is 0 100644 Umask is 200 100444 Umask is 400 100644 Umask is 600 100444 If you run it, it will create 4 files (for the 4 tested modes) so you can see the permissions applied. I looked into adding this support to File.open, but I’m not certain where to begin (I found public RuleGenerator Open(), which I don’t know enough about yet). JD From: Tomas M. Sent: Wednesday, April 22, 2009 12:00 AM To: Jim D.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec What is umask supposed to do? Tomas From: Jim D. Sent: Tuesday, April 21, 2009 8:12 PM To: removed\_email\_address@domain.invalid; IronRuby External Code R. Subject: Code Review: Update RubySpec I’m updating RubySpec to the newest revision from Git. Most of this is boring stuff; just updating files and rebuilding tags. However, a couple of these commits are code changes to help the specs run smoother. JD - ``` add ci_files to default.mspec so mspec ci ``` works[http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a](http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a) - ``` add a filtered function to mspec to simplify ``` default.mspec[http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2](http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2) * * * - ``` categorizing uncategorized ``` specs[http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657](http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657) * * * - ``` removing extra spec ``` files[http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6](http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6) - ``` update rubyspec and ``` mspec[http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e](http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e) - ``` add a file with the git sha1 hashes of current mspec and ``` rubyspec[http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73](http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73) - ``` implement File.umask in order to run the ``` specs[http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef](http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef) * * * - ``` Implement Process.waitall (needed by Process ``` specs)[http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e](http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e) * * * - ``` rebuilt ironruby ``` tags[http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475](http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475) The commits marked with \*\*\* are more than just infrastructure work. These are actual code changes that need review. Thanks JD ---
**Author:** ![raker\_t](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/r/ecae2f/32.png) [@raker\_t](https://www.ruby-forum.com/u/raker_t)\ **Post date:** [April 22, 2009, 9:10pm UTC](https://www.ruby-forum.com/t/code-review-update-rubyspec/166129/5 "2009-04-22T21:10:06Z")
I see - umask maintains a global state. Any global state maintained by libraries should be stored on RubyContext, not in static fields. Use RubyContext.GetOrCreateLibraryData/TryGetLibraryData (if you search the library you’ll find some use cases). Also defining a struct just to store an integer seems unnecessary (besides it’s not good to define structs with mutable state). Tomas From: Jim D. Sent: Wednesday, April 22, 2009 11:09 AM To: Tomas M.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec Umask is a mask on file creation. By itself it does nothing more than what I’ve implemented (that I can tell), but File.open (and other file creation methods) should look at it to derive the default permissions. The attached script shows the basic idea, and here’s it’s output: [9] \> ruby .\temp.rb Umask is 0 100644 Umask is 200 100444 Umask is 400 100644 Umask is 600 100444 If you run it, it will create 4 files (for the 4 tested modes) so you can see the permissions applied. I looked into adding this support to File.open, but I’m not certain where to begin (I found public RuleGenerator Open(), which I don’t know enough about yet). JD From: Tomas M. Sent: Wednesday, April 22, 2009 12:00 AM To: Jim D.; removed\_email\_address@domain.invalid; IronRuby External Code Reviewers Subject: RE: Code Review: Update RubySpec What is umask supposed to do? Tomas From: Jim D. Sent: Tuesday, April 21, 2009 8:12 PM To: removed\_email\_address@domain.invalid; IronRuby External Code R. Subject: Code Review: Update RubySpec I’m updating RubySpec to the newest revision from Git. Most of this is boring stuff; just updating files and rebuilding tags. However, a couple of these commits are code changes to help the specs run smoother. JD - ``` add ci_files to default.mspec so mspec ci ``` works[http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a](http://github.com/jredville/ironruby/commit/2ef5e803b6089a3b7b6513de8eb44555f8a8537a) - ``` add a filtered function to mspec to simplify ``` default.mspec[http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2](http://github.com/jredville/ironruby/commit/eb9d2047ecee954205fb1d8cd80df7ffaeb4bac2) * * * - ``` categorizing uncategorized ``` specs[http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657](http://github.com/jredville/ironruby/commit/0845a3e803688372d9f1959633ad7840f82f7657) * * * - ``` removing extra spec ``` files[http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6](http://github.com/jredville/ironruby/commit/d973948111456d387e91243e4d8399483fb29fc6) - ``` update rubyspec and ``` mspec[http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e](http://github.com/jredville/ironruby/commit/75122c3ade440e28d933c658a95b43acd26e5f9e) - ``` add a file with the git sha1 hashes of current mspec and ``` rubyspec[http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73](http://github.com/jredville/ironruby/commit/588dcbfa225156c19c61d882f82e515c5b8f0e73) - ``` implement File.umask in order to run the ``` specs[http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef](http://github.com/jredville/ironruby/commit/61ce27f27a510c512095aa3b5ab8329229dce7ef) * * * - ``` Implement Process.waitall (needed by Process ``` specs)[http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e](http://github.com/jredville/ironruby/commit/f1f148f773111c561e1eea515681799a7559437e) * * * - ``` rebuilt ironruby ``` tags[http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475](http://github.com/jredville/ironruby/commit/d176ec1d4e6d546420e6645554b75c3f94d8e475) The commits marked with \*\*\* are more than just infrastructure work. These are actual code changes that need review. Thanks JD