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

Added features to whois.ua parser #169

Merged
merged 12 commits into from Jun 13, 2012
Merged

Added features to whois.ua parser #169

merged 12 commits into from Jun 13, 2012

Conversation

Uko
Copy link
Contributor

@Uko Uko commented May 29, 2012

Added property_supported:

  1. domain
  2. admin_contacts
  3. technical_contacts

Added property_not_supported:

  1. domain_id
  2. referral_whois
  3. referral_url
  4. registrar
  5. registrant_contacts

Implemented property:

  1. created_on

Added one more test for registered domain.

property_supported :created_on do
if content_for_scanner =~ /created:\s+(.+)\n/
time = $1.split(" ").last
begin
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the case where the parsing fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:updated_on and :expires_on parse sections that existed before I've forked a repo, worked exactly the same. Should I do something special here?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that originally there was an invalid date. This case doesn't seem to exist anymore so I would simply change the code to

time = $1.split(" ").last
Time.parse(time)

without the begin/rescue statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be done

@weppos
Copy link
Owner

weppos commented Jun 13, 2012

Hi @Uko, the patch looks good to me. I only have a couple of requests

  1. please order the properties both in the parser and in the specs according to the other they have in the other parsers. For instance, in the specs I can see registrar is defined after contacts. You can use any parser as example, such as whois.nic.it.
  2. what is the difference between registered and registered1 ?

@Uko
Copy link
Contributor Author

Uko commented Jun 13, 2012

  1. I will reorder properties.
  2. 2nd has two technical contacts and I wanted to be sure that the parser will find both. Also the address is in different format, so I've tested on deferent addresses.

@weppos
Copy link
Owner

weppos commented Jun 13, 2012

  1. Great! Just please rename everything from status_registered1 to property_contacts_multiple. Thanks!

@Uko
Copy link
Contributor Author

Uko commented Jun 13, 2012

Looks like I've done everything you asked

@ghost ghost assigned weppos Jun 13, 2012
weppos added a commit that referenced this pull request Jun 13, 2012
Added features to whois.ua parser
@weppos weppos merged commit 73f971e into weppos:master Jun 13, 2012
weppos added a commit that referenced this pull request Jun 13, 2012
@weppos
Copy link
Owner

weppos commented Jun 13, 2012

Merged, thank you! I just recommend you to change your editor settings to use spaces instead of tabs.

Thanks again for your patch.

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

2 participants