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

Fix List Records command and add test #10

Merged
merged 1 commit into from
Feb 12, 2012
Merged

Fix List Records command and add test #10

merged 1 commit into from
Feb 12, 2012

Conversation

johnbintz
Copy link
Contributor

The List Records command was broken, as it was not sending a DNSimple::Domain instance to Records.all. I fixed this, made the class subclass DNSimple::Command to work like other commands, and added a simple spec to ensure basic data was being printed. The tests will need some additional cleanup, and a Cucumber feature will eventually need to be added, but the command works for me now.

@troy
Copy link

troy commented Feb 12, 2012

I just fixed this as well before finding this pull request. @aeden @dje: could one of you merge @johnbintz's patch or my one-liner below and release a new gem? It's a bug that that most CLI users will encounter (http://support.dnsimple.com/discussions/questions/92-cli-dnsimple-recordlist-raises-nomethoderror-exception).

My one-line fix was to replace:

domain_name = args.shift

with:

domain_name = Domain.find(args.shift)

.. so domain_name is a Domain instead of a String. John's is probably cleaner and definitely has better coverage.

aeden added a commit that referenced this pull request Feb 12, 2012
Fix List Records command and add test
@aeden aeden merged commit 16049c0 into dnsimple:master Feb 12, 2012
@aeden
Copy link
Member

aeden commented Feb 12, 2012

Done. New version is 1.2.5. Thanks for keeping on me about it.

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.

None yet

4 participants