Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve debug experience for MergedConfig #8

Merged
merged 2 commits into from
Aug 22, 2014
Merged

improve debug experience for MergedConfig #8

merged 2 commits into from
Aug 22, 2014

Conversation

mwrock
Copy link
Member

@mwrock mwrock commented Jun 20, 2014

On a couple occasions while debugging my metal driver, I've been perplexed by errors such as:

#<ArgumentError: wrong number of arguments (2 for 1)> with backtrace:
         # /home/mwrock/dev/cheffish/lib/cheffish/merged_config.rb:48:in `method_missing'
         # /home/mwrock/dev/cheffish/spec/functional/merged_config_spec.rb:14:in `block (3 levels) in <top (required)>'

The root of this is bad code on my end where, for example, I think I have a normal hash but I really have a MergedConfig and calling merge(some_hash) results in an error like the one above. I spend several moments trying to figure out where I am passing in the wrong number of args and then after looking at the stack more carefully, I open up merged_config.rb to realize this is a side effect of my error.

This PR tries to make the initial error more clear.

Also, when debugging, I also run into situations where I have stuck in code like:

puts "my options:#{config[:driver_options]}"

I know...I know terrible debugging technique but I'm a ruby noob and sometimes this is what I do.

This tells me something rather unhelpful:

my options:<Cheffish::MergedConfig:0x000000010cba38>

This PR changes the string representation of MergedConfig to use its to_hash and then return the to_s of that hash. This helps me to see what the merge_config is holding.

@chef-supermarket
Copy link

Hi. Your friendly Curry bot here. Just letting you know that there are commit authors in this Pull Request who appear to not have signed a Chef CLA.

The following GitHub users do not appear to have signed a CLA:

Please sign the CLA here.

@mwrock
Copy link
Member Author

mwrock commented Jul 15, 2014

signed CLA

@jkeiser
Copy link
Contributor

jkeiser commented Aug 22, 2014

Looks super reasonable to me! I missed this repo when I caught up after vacation, sorry for the lateness.

jkeiser added a commit that referenced this pull request Aug 22, 2014
improve debug experience for MergedConfig
@jkeiser jkeiser merged commit 3090bbd into chef:master Aug 22, 2014
@mwrock
Copy link
Member Author

mwrock commented Aug 23, 2014

Sweet. Thanks John!

@thommay thommay added Improvement Type: Enhancement Adds new functionality. and removed Improvement labels Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants