New GoDaddy parser and fix issue 103 #105

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

pmyteh commented Jul 31, 2011

I've written a simple parser for responses from whois.godaddy.com, the single largest .com registry. Their format for addresses is depressingly irregular (may be missing the organization and/or telephone/fax number lines, without the helpful field: data format given by some other registries) but it works pretty well and adds parseability to ~30% of all .coms.

Issue #103 is also fixed by a separate small commit.

Thanks,
Tom

Owner

weppos commented Aug 4, 2011

Hi Tom,

thank you very much for the parser. Unfortunately, I cannot merge the pull request without a test suite.
Would it be possible to provide it?

Creating a test suite for a parser is just easy as extracting some fixtures and creating a .expected file.
See for example bfbd27b or 8218ebb

Owner

weppos commented Aug 4, 2011

I merged your fix for issue #103

@ghost ghost assigned weppos Aug 4, 2011

Contributor

pmyteh commented Aug 4, 2011

Sorry - I'm rather new to this. I'll have a look and put something together.

Contributor

pmyteh commented Aug 5, 2011

Hope that's sufficient. Like whois.markmonitor.com, the server really shouldn't be queried unless the domain is registered. If there's something else I'm missing, do let me know: I'd like to get this right.

Owner

weppos commented Aug 16, 2011

Thanks @pmyteh. Once you create the tests, you should also run

$ rake genspec:parser

to generate the test suites and then run

$ rake

to execute them. I executed the test suite and one test fails.

1) Whois::Record::Parser::WhoisGodaddyCom status_registered.expected#registrant_contacts 
   Failure/Error: @parser.registrant_contacts[0].email.should        == ""
     expected: ""
          got: nil (using ==)
   # ./spec/whois/record/parser/responses/whois.godaddy.com/status_registered_spec.rb:80:in `block (3 levels) in <top (required)>'

The rest of the patch is ok for me.

@weppos weppos closed this in fae71f9 Aug 25, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment