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

remove kubernetes related option #125

Merged
merged 1 commit into from
Apr 9, 2020
Merged

Conversation

zouyee
Copy link
Member

@zouyee zouyee commented Apr 5, 2020

Signed-off-by: Zou Nengren [email protected]

notes for reviewer:

Also re-enabled and adjusted the kubernetes upstream behavior test to verify default upstream 
behavior. The test had been commented out when the upstream option was originally deprecated.

coredns/coredns#3737

@zouyee zouyee force-pushed the remove branch 2 times, most recently from 1c33dfb to 2264812 Compare April 5, 2020 15:22
@zouyee
Copy link
Member Author

zouyee commented Apr 6, 2020

/cc @miekg

@miekg
Copy link
Member

miekg commented Apr 6, 2020

'ci' is pulled down from circle-ci to test, are we sure this works? @chrisohaver

@chrisohaver
Copy link
Member

Looks ok. However, there is more that is being changed here than just the removal of upstream option, which warrants explanation. Even if it seems obvious now, it may not be to others in the future.

@zouyee, please write a description.

@zouyee
Copy link
Member Author

zouyee commented Apr 9, 2020

/assign @chrisohaver

@chrisohaver
Copy link
Member

chrisohaver commented Apr 9, 2020

My apologies, what I meant was to add a description to the PR here in github, not to add comments to each changed line of code.

Something like ... "Also re-enabled and adjusted the kubernetes upstream behavior test to verify default upstream behavior. The test had been commented out when the upstream option was originally deprecated."

Can you remove the comments added in 1c186d6, I don't think they are needed. Thanks!

Signed-off-by: zounengren <[email protected]>
@chrisohaver chrisohaver merged commit d5e3a35 into coredns:master Apr 9, 2020
@zouyee zouyee deleted the remove branch April 10, 2020 01:12
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.

3 participants