Skip to content

Refactor Resolver.create_tsig_options#69

Merged
alexdalitz merged 3 commits into
alexdalitz:masterfrom
elibus:fix-create_tsig_options
Feb 17, 2015
Merged

Refactor Resolver.create_tsig_options#69
alexdalitz merged 3 commits into
alexdalitz:masterfrom
elibus:fix-create_tsig_options

Conversation

@elibus
Copy link
Copy Markdown
Contributor

@elibus elibus commented Feb 15, 2015

  • Fix: options[:algorithm] was NOT an assignment
  • Refactor arguments
  • Add tests

 * Fix: options[:algorithm] was NOT an assignment
 * Refactor arguments
 * Add tests
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 76.82% when pulling 714338b on elibus:fix-create_tsig_options into d2680e9 on alexdalitz:master.

@keithrbennett
Copy link
Copy Markdown
Contributor

Hi, Marco. Thanks for those changes.

I believe create_tsig_options no longer checks for argument list size since the regular Ruby method parameter approach is now being used. So the tests that test for ArgumentError failed for me.

I've posted suggested changes at https://gist.github.com/keithrbennett/fac9ffada155680ebea9. Basically, the create_tsig_options method can be simplified, and the test should no longer contain anything regarding argument list size checking.

If you (and Alex) agree with those changes, please incorporate them into your feature branch. I tried forking your repo to issue a pull request to it, but Github couldn't handle it because it would then have had the same name as my fork of Alex's. What a complicated life we live. ;)

Grazie!

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 76.76% when pulling 93d5248 on elibus:fix-create_tsig_options into d2680e9 on alexdalitz:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0%) to 76.74% when pulling b03005c on elibus:fix-create_tsig_options into d2680e9 on alexdalitz:master.

alexdalitz added a commit that referenced this pull request Feb 17, 2015
Refactor Resolver.create_tsig_options
@alexdalitz alexdalitz merged commit 1dec3ad into alexdalitz:master Feb 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants