Types spike #1
Merged
Types spike #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* This should not be merged into master as-is. This is mainly being committed so that the basic API design can be finalized before progressing too far in the wrong direction.
* Currently adamantium isn't being used by the library, while ice_nine is.
* In Ruby 1.9 BasicObject is the top-most object, and it does not have #kind_of? so we use BasicObject#=== instead.
| # | ||
| # @api private | ||
| def define_option_method(option) | ||
| ivar = "@#{option}".freeze |
There was a problem hiding this comment.
You begun to freeze local immutable variables. For the reason the later closures should not have a chance to modify it?
There was a problem hiding this comment.
I suppose it does not make a difference here, since I control what happens in the closure, and I can just not mutate the string. It's not like this local variable could ever be accessible outside this scope.
* This allows the constraint to work with other types and not raise a NoMethodError, but rather return false like it is supposed to.
* The type will be deep frozen during finalization, so it's fine to not freeze here. Plus freezing the local variable is unnecessary.
* Add backports as an explicit runtime dependency since this code makes use of 1.9 core lib features. * Remove equalizer since it is not being used (yet). * Add development dependencies to gemspec. While they are specified in the Gemfile, I prefer for runtime and development dependencies required to run the specs and generate docs to be in the gemspec.
* The finalize methods sometimes update the type state, but once the type has been finalized the work should not be repeated. When the type has been frozen it is an indication it has been finalized.
* In 1.9 ::Object.superclass will return ::BasicObject, under 1.8 it will return nil, so this will default to ::Object
* This change is the most cross platform compatible change, and if limits are hit it should be possible to define different limits for different kinds of systems. I did some basic research and it appears that different rubies have different limits. I could not get any consensus about what the limits are, if any. Ideally there would be a formula that could be used to determine the platform specific limits; but I'm guessing that it will need to be specific using conditional logic or a case statement at best. For now, default to the lowest common denominator until it becomes an issue or per-platform limits are known.
* I benchmarked the difference between Array#include? and Set#include? and the latter was faster even for small sets of objects. The performance was outstanding for larger sets. It makes sense since Array#include? is O(n) and Set#include? is O(1), yet I thought I has read somewhere that Array#include? was faster when "n" was small, but that does not seem to be the case, at least under ruby 1.9.3.
* Internal methods now act on a single option at a time for the most part, allowing one loop in accept_options and then delegation to private methods for object setup. * There appears to be one block of code that I cannot cover with mutant. I feel like it is still necessary so I will be exploring ways to cover it.
* Remove dead code path
* Identified one missing case which was fixed by the formerly removed "dead code path", which turned out to be needed anyway.
* I wish there was a #protected_send method I could use here.
* For some reason the block passed to Class.new is not being evaluated under 1.8.7, causing the specs to fail.
* Update to use a simpler way to check to see if the methods are responded to. The previous way fails in jruby.
* The documented return value is a Boolean, and that should be enough. Any return value besides a Boolean is not guaranteed to work, and in the future may throw exceptions should type checking based on the YARD docs ever be implemented in the specs.
* Only use the jruby sections where I can turn on the debug flag to get proper coverage reports back.
* The simplecov coverage will not execute under 1.8 so there is no point in having this configured. * Keep jruby-18mode in the matrix/include list so that it will be grouped with the other jruby results.
* Remove stub Encoding class from spec support, and test to see if the constant is defined before using it in the encodable spec.
|
Changes Unknown when pulling 85313dd on types-spike into * on master*. |
|
Changes Unknown when pulling 85313dd on types-spike into * on master*. |
|
Changes Unknown when pulling 85313dd on types-spike into * on master*. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_DO NOT MERGE INTO MASTER_
The purpose of opening a pull request is to solicit feedback on the API design from @solnic, @snusnu, @mbj and others. This should not be merged into master in it's current form.
Once the API has been discussed, I will develop the code using TDD and metrics driven development like other DataMapper 2 components.