Page MenuHomePhabricator

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

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

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC76868acef7b3: Move parse_name() to dns.h so it can be tested
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Mon, Oct 21, 19:34
Owners added a reviewer: Restricted Owners Package.Mon, Oct 21, 19:34
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Oct 21, 19:34
nakihito planned changes to this revision.Mon, Oct 21, 19:34
nakihito updated this revision to Diff 13631.Mon, Oct 21, 19:39

Added formatting changes to this patch.

nakihito planned changes to this revision.Mon, Oct 21, 19:39
nakihito requested review of this revision.Mon, Oct 21, 19:40
jasonbcox requested changes to this revision.Mon, Oct 21, 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.Mon, Oct 21, 20:38
nakihito added inline comments.Tue, Oct 22, 00:20
src/seeder/dns.h
34 ↗(On Diff #13631)

I would have to remove static if I did.

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

and?

This revision now requires changes to proceed.Tue, Oct 22, 13:11
nakihito updated this revision to Diff 13675.Wed, Oct 23, 20:05

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

nakihito planned changes to this revision.Wed, Oct 23, 20:12
nakihito requested review of this revision.Fri, Oct 25, 02:27
jasonbcox requested changes to this revision.Fri, Oct 25, 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.Fri, Oct 25, 18:24
nakihito updated this revision to Diff 13713.Fri, Oct 25, 20:40

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

nakihito planned changes to this revision.Fri, Oct 25, 22:18
nakihito updated this revision to Diff 13715.Fri, Oct 25, 22:21

Removed changes to comments.

jasonbcox requested changes to this revision.Sat, Oct 26, 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.Sat, Oct 26, 00:45
nakihito updated this revision to Diff 13718.Sat, Oct 26, 01:09

Removed cstring for cstddef for size_t.

jasonbcox accepted this revision.Sun, Oct 27, 17:14
deadalnix accepted this revision.Mon, Oct 28, 02:48
This revision is now accepted and ready to land.Mon, Oct 28, 02:48