Page MenuHomePhabricator

Move parse_name() to dns.h so it can be tested
ClosedPublic

Authored by nakihito on Oct 21 2019, 19:34.

Details

Summary

parse_name() is one of two functions are critical to all other dns functions
and should be tested. It must be moved to dns.h in order to avoid including
dns.cpp.

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
MoveParseName
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7824
Build 13670: Bitcoin ABC Buildbot (legacy)
Build 13669: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 21 2019, 19:34

Added formatting changes to this patch.

jasonbcox requested changes to this revision.Oct 21 2019, 20:38
jasonbcox added inline comments.
src/seeder/dns.h
34 ↗(On Diff #13631)

Why not just declare it?

This revision now requires changes to proceed.Oct 21 2019, 20:38
src/seeder/dns.h
34 ↗(On Diff #13631)

I would have to remove static if I did.

deadalnix requested changes to this revision.Oct 22 2019, 13:11
deadalnix added inline comments.
src/seeder/dns.h
34 ↗(On Diff #13631)

and?

This revision now requires changes to proceed.Oct 22 2019, 13:11

Removed static from parse_name() and seperated the declaration from definiton.

jasonbcox requested changes to this revision.Oct 25 2019, 18:24

This diff is doing at least 3 things that aren't in the summary or title. Why not do nit-fixes in a separate diff and make this an easily reviewable, small change?

This revision now requires changes to proceed.Oct 25 2019, 18:24

Removed linter changes to comment spacing and removed braces that were added.

Removed changes to comments.

jasonbcox requested changes to this revision.Oct 26 2019, 00:45
jasonbcox added inline comments.
src/seeder/dns.h
5 ↗(On Diff #13715)

why is this added?

This revision now requires changes to proceed.Oct 26 2019, 00:45

Removed cstring for cstddef for size_t.

This revision is now accepted and ready to land.Oct 28 2019, 02:48