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

plugin/forward: various cleanup #1949

Merged
merged 1 commit into from
Jul 7, 2018
Merged

plugin/forward: various cleanup #1949

merged 1 commit into from
Jul 7, 2018

Conversation

miekg
Copy link
Member

@miekg miekg commented Jul 7, 2018

Fix documentation and remove the unused From method.

Signed-off-by: Miek Gieben miek@miek.nl

Fix documentation and remove the unused From method.

Signed-off-by: Miek Gieben <miek@miek.nl>
@corbot corbot bot requested a review from johnbelamaric July 7, 2018 13:06
@corbot
Copy link

corbot bot commented Jul 7, 2018

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked johnbelamaric (via plugin/forward/OWNERS) for a review.

@codecov-io
Copy link

codecov-io commented Jul 7, 2018

Codecov Report

Merging #1949 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1949      +/-   ##
==========================================
+ Coverage   54.43%   54.44%   +<.01%     
==========================================
  Files         197      197              
  Lines        9574     9572       -2     
==========================================
- Hits         5212     5211       -1     
+ Misses       3957     3956       -1     
  Partials      405      405
Impacted Files Coverage Δ
plugin/forward/forward.go 56.47% <0%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41c2871...2891c7c. Read the comment docs.

@miekg
Copy link
Member Author

miekg commented Jul 7, 2018

/merge

@miekg miekg merged commit 6ec1978 into master Jul 7, 2018
@corbot corbot bot deleted the docs branch July 7, 2018 13:38
@miekg miekg removed the request for review from johnbelamaric July 7, 2018 13:38
@rdrozhdzh
Copy link
Contributor

@miekg, the method Forward.From() was requested in #1762, and added in #1766. It may be still needed.
@fturib, @ekleiner, could you confirm?

@miekg
Copy link
Member Author

miekg commented Jul 7, 2018

Ah yes, damn, I remember. Something, something that needed this. If there are no in-tree users of exported functions this is not sustainable (and there is no backwards compatibility guarantee)

@fturib
Copy link
Contributor

fturib commented Jul 9, 2018

@rdrozhdzh : thank you for reminding these requests.
After check in our ATEPMonitor plugin, I think we are not using it anymore.

@miekg : what do you mean by "in-tree users of exported functions" ? I mean, how should we code the functions we need to be public for external plugins such a way they will not be deleted in refactoring ?

@miekg
Copy link
Member Author

miekg commented Jul 9, 2018 via email

mdgreenfield pushed a commit to DataDog/coredns that referenced this pull request Mar 4, 2019
Fix documentation and remove the unused From method.

Signed-off-by: Miek Gieben <miek@miek.nl>
Jason-ZW pushed a commit to rancher/coredns that referenced this pull request Apr 17, 2019
Fix documentation and remove the unused From method.

Signed-off-by: Miek Gieben <miek@miek.nl>
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

4 participants