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

Add support for Windows DNS nameserver configuration #40

Merged
merged 10 commits into from
Jan 4, 2017
Merged

Conversation

kelunik
Copy link
Member

@kelunik kelunik commented Jan 4, 2017

This drops the default Google nameservers, as they were only required for Windows.

Fixes #38.

This drops the default Google nameservers, as they were only required for Windows.

Fixes #38.
@kelunik kelunik requested a review from bwoebi January 4, 2017 11:32
@mention-bot
Copy link

@kelunik, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bwoebi, @rdlowrey and @DaveRandom to be potential reviewers.

@kelunik
Copy link
Member Author

kelunik commented Jan 4, 2017

I added the default servers back, but issue a deprecation warning instead now. That allows creating a minor release now and removing them in the next major release. I don't want to change all packages now to upgrade to a new major for that one.

$reader = new WindowsRegistry;
$nameserver = null;

while ($key = array_shift($keys)) {
Copy link
Member

Choose a reason for hiding this comment

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

Just foreach?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't really matter.

Copy link
Member

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Please do not remove default nameservers.
Or is there any reason why deprecating these?

@kelunik
Copy link
Member Author

kelunik commented Jan 4, 2017

@bwoebi Every known system should work now. If there's one that doesn't work, we should be made aware of it and fix it. A broken config should error, not simply send all your DNS queries to Google.

@kelunik kelunik mentioned this pull request Jan 4, 2017
@bwoebi bwoebi merged commit cf08357 into master Jan 4, 2017
@kelunik
Copy link
Member Author

kelunik commented Jan 4, 2017

Args, a bit too fast...

peter279k pushed a commit to peter279k/dns that referenced this pull request Dec 7, 2020
Alpine doesn't have dig by default. Thought I'd add it to the doc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants