Skip to content
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Conversation

@majain-vrsn
Copy link

Please review this pull request, which implements a Denominator provider for Verisign’s managed DNS service. It includes basic zone and resource record management operations. It represents a substantial rewrite of previous submissions based on prior comments, and the new implementation is based on an existing provider implementation. Please let us know your feedback.

@cloudbees-pull-request-builder

NetflixOSS » denominator » denominator-pull-requests #142 FAILURE
Looks like there's a problem with this pull request

Copy link
Contributor

Choose a reason for hiding this comment

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

this class seems to repeat work done in the normal live tests. also, it is storing XML, which seems distracting. Should this become a mock test? or should we remove it?

@codefromthecrypt
Copy link
Contributor

did a very cursory review. I might have an account to test with, still. Can do more review in a day or two.

@codefromthecrypt
Copy link
Contributor

PS this looks much cleaner than before, nice work.

Before we merge this, please review this and make sure qualifications are met. For example, we've not had any user ask for this. It may make sense to host this in your repository until there's diverse need, similar to how we support discoverydns.

https://github.com/Netflix/denominator/wiki/Adding-new-Providers

@cloudbees-pull-request-builder

NetflixOSS » denominator » denominator-pull-requests #143 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » denominator » denominator-pull-requests #144 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » denominator » denominator-pull-requests #145 SUCCESS
This pull request looks good

@mcarrer
Copy link

mcarrer commented Dec 15, 2015

All, we tested the Denominator provider for Verisign’s managed DNS in our product and it worked fine.
We would like to have this merged upstream for a better management of the dependencies.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants