Separate name from ruby_version attributes to fix #49.

[freerobby]

This fixes #49.

[KAllan357]

I like the separation of name and ruby_version, but I think the code could be cleaner (unless I'm misunderstanding). Consider the following examples:

#1
rbenv_ruby "Ruby 1.9.3" do
  ruby_version "1.9.3-p448"
end

#2
rbenv_ruby "1.9.3-p448"

I think the code should consider using a ternary. If name is set, but ruby_version is unset, just use name and you'll get an error when you get cute like in example 1. When both are set, assume name is the cute stuff and ruby_version is the good stuff. Example 2 is for when you give name the good stuff.

What do you think about modeling that kind of behavior?

[freerobby]

@KAllan357 I'm torn here. I think the cleanest approach would be not to induce anything from the name, and require a ruby_version attribute. The downside to that approach is that it would be a breaking change.

Barring a special use case, I would advocate for leaving the ruby_version as the name_attribute and disallowing separation between names and ruby_versions. But it just so happens I have a use case that requires it:

ruby_block 'detect-application-ruby-version' do
  block do
    $application_ruby_version = `/usr/bin/head -n 1 #{APP_ROOT}/.ruby-version`.chomp
  end
end

rbenv_ruby 'application-ruby-version' do
  ruby_version lazy { $application_ruby_version }
  global true
end

In this example, the ruby version is not known until execution time, but a name must be supplied at compile time, so they need to be separate attributes.

With respect to a ternary operator, are you thinking of line 33 of resources/ruby.rb or do you mean in providers/ruby.rb? I think the ||= operator is roughly as clean as a ternary operator but I don't have strong feelings. I do think that the change should be made in the resource rather than the provider so that the attributes have consistent meaning in the underlying provider code regardless of how the user sets them.

With that said, I'm happy to proceed here however you feel is best. Just let me know and I'll be happy to update the code accordingly.

[KAllan357]

@freerobby Oh wow, that is an interesting example. After thinking about it some more, I think the best bet here is to keep this as is - fix the bug, and it can be refactored later to reflect a cleaner syntax / usage of the provider.

[freerobby]

@KAllan357 Sounds good. Thanks for looking everything over thoroughly and for the sanity checks. If you do decide you'd like something done differently here, just give a yell.

[freerobby]

@KAllan357 Anything I can do to get this merged into master.

[KAllan357]

:+1: LGTM

[KAllan357]

@freerobby I'm thinking this might necessitate a major release (backwards incompatible change). Do you agree, or is this actually bw compatible?