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

Fixes #18398: Use consistent DNS parameter naming #507

Closed
wants to merge 1 commit into from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Feb 4, 2017

  • fqdn is what is defined in the API and should only be in dns_api.rb
  • value is what is defined in the API and should only be in dns_api.rb
  • name is the DNS name you query for
  • content is the result your DNS server should return

This should avoid confusion with types where the content is also a valid
hostname so fqdn is ambigious.

@theforeman-bot
Copy link
Member

There were the following issues with the commit message:

  • 94919bd must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

* fqdn is what is defined in the API and should only be in dns_api.rb
* value is what is defined in the API and should only be in dns_api.rb
* name is the DNS name you query for
* content is the result your DNS server should return

This should avoid confusion with types where the content is also a valid
hostname so fqdn is ambigious.
@ekohl ekohl changed the title Use consistent DNS parameter naming Fixes #18398: Use consistent DNS parameter naming Feb 4, 2017
@dmitri-d
Copy link
Member

dmitri-d commented Feb 6, 2017

I think that in most places these changes made method signatures more confusing: def create_a_record(fqdn, ip) is much clearer about its parameter expectations compared to def create_a_record(name, content).

Perhaps instead of trying to come up with a common parameter naming convention we should rename parameters in cname-related method to more descriptive ones? For example: def create_cname_record(existing_fqdn, new_alias)?

@ekohl
Copy link
Member Author

ekohl commented Feb 14, 2017

I agree with it on create_a_record that it's less readable, but my reasoning is that I want the Record class to be much closer to actual DNS. I'll first try to get other refactorings (like #511) in and then circle back here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants