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

feat: replace remaining Box<dyn DnsRecordExt> with type #271

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

CosminPerRam
Copy link
Contributor

#206 replaced some remaining Box<dyn DnsRecordExt> in members and params, this PR replaces the one in the return position of DnsRecordExt::clone_box (and subsequently in each impl), with this there will be only one Box<> usage, which is the type definition.

One might uphold that having the full definition in the return position to be better, but I see it as an arbitrary choice as everywhere else we use the type.

@CosminPerRam
Copy link
Contributor Author

I have also taken the liberty to move the Clone impl closer to the type definition to make it more visible when browsing the code, if this is not seen as appropiate, please mention if so to revert it.

Copy link
Owner

@keepsimple1 keepsimple1 left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! LGTM!

@keepsimple1 keepsimple1 merged commit 39acd80 into keepsimple1:main Nov 19, 2024
3 checks passed
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.

2 participants