starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 03:05:51 +03:00

[GH-ISSUE #167] Borrow `Handle`s, don't consume them #79

New issue

Closed

opened 2026-03-07 22:18:37 +03:00 by kerem · 5 comments

kerem commented

2026-03-07 22:18:37 +03:00

Owner

Originally created by @rushmorem on GitHub (Jul 20, 2017).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/167

I noticed a number of methods (eg UdpStream::new) consume tokio_core::reactor::Handle instead of simply borrowing it since the underlying methods only require references. This results in unnecessary cloning in client code.

Originally created by @rushmorem on GitHub (Jul 20, 2017). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/167 I noticed a number of methods (eg `UdpStream::new`) consume `tokio_core::reactor::Handle` instead of simply borrowing it since the underlying methods only require references. This results in unnecessary cloning in client code.

kerem closed this issue

2026-03-07 22:18:38 +03:00

kerem commented

2026-03-07 22:18:39 +03:00

Author

Owner

@bluejekyll commented on GitHub (Jul 20, 2017):

Hmm. Did this change in recent Tokio versions?

I think AsRef as the parameter change would fix this without creating a breaking change.

@bluejekyll commented on GitHub (Jul 20, 2017): Hmm. Did this change in recent Tokio versions? I think AsRef<Handle> as the parameter change would fix this without creating a breaking change.

kerem commented

2026-03-07 22:18:39 +03:00

Author

Owner

@rushmorem commented on GitHub (Jul 20, 2017):

Did this change in recent Tokio versions?

As long as I remember, this has always been the case. Methods in Tokio have always borrowed Handle.

@rushmorem commented on GitHub (Jul 20, 2017): > Did this change in recent Tokio versions? As long as I remember, this has always been the case. Methods in Tokio have always borrowed `Handle`.

kerem commented

2026-03-07 22:18:39 +03:00

Author

Owner

@bluejekyll commented on GitHub (Jul 20, 2017):

Huh. I don't know why I would have made that mistake. I'm on a family vacation right now. I might be able to get to this at some point this week. Otherwise a PR is always welcome.

@bluejekyll commented on GitHub (Jul 20, 2017): Huh. I don't know why I would have made that mistake. I'm on a family vacation right now. I might be able to get to this at some point this week. Otherwise a PR is always welcome.

kerem commented

2026-03-07 22:18:39 +03:00

Author

Owner

@rushmorem commented on GitHub (Jul 20, 2017):

I think AsRef as the parameter change would fix this without creating a breaking change.

IMHO it might actually be desirable to make this a breaking change otherwise people might not be aware of it.

I'm on a family vacation right now. I might be able to get to this at some point this week. Otherwise a PR is always welcome.

Don't worry about it. Please enjoy your vacation. I will work on this and submit a pull request.

@rushmorem commented on GitHub (Jul 20, 2017): > I think AsRef as the parameter change would fix this without creating a breaking change. IMHO it might actually be desirable to make this a breaking change otherwise people might not be aware of it. > I'm on a family vacation right now. I might be able to get to this at some point this week. Otherwise a PR is always welcome. Don't worry about it. Please enjoy your vacation. I will work on this and submit a pull request.

kerem commented

2026-03-07 22:18:39 +03:00

Author

Owner

@bluejekyll commented on GitHub (Jul 20, 2017):

make this a breaking change otherwise people might not be aware of it.

Yeah. I thought about that. It makes sense, I'll plan on a 0.11 release for this in a little bit in that case.

And thanks for the PR!

@bluejekyll commented on GitHub (Jul 20, 2017): > make this a breaking change otherwise people might not be aware of it. Yeah. I thought about that. It makes sense, I'll plan on a 0.11 release for this in a little bit in that case. And thanks for the PR!

kerem referenced this issue

2026-03-15 22:03:56 +03:00

[GH-ISSUE #79] Move client integration tests back from server/ to client/ after split #342

kerem referenced this issue

2026-03-16 01:58:21 +03:00

[PR #187] [MERGED] Resolver: Cname chaining #1288