Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING76868acef7b3: Move parse_name() to dns.h so it can be tested
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

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

nakihito created this revision.Oct 21 2019, 19:34

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

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 21 2019, 19:34

nakihito planned changes to this revision.Oct 21 2019, 19:34

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing failed.

Harbormaster failed remote builds in B7824: Diff 13629!Oct 21 2019, 19:36

Added formatting changes to this patch.

nakihito planned changes to this revision.Oct 21 2019, 19:39

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito requested review of this revision.Oct 21 2019, 19:40

nakihito added a child revision: D4276: Add unit tests for seeder's parse_name().Oct 21 2019, 19:45

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7825: Diff 13631.Oct 21 2019, 19:49

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

nakihito added inline comments.Oct 22 2019, 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.Oct 22 2019, 00:45

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.

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito planned changes to this revision.Oct 23 2019, 20:12

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7856: Diff 13675.Oct 23 2019, 20:15

nakihito requested review of this revision.Oct 25 2019, 02:27

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.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7876: Diff 13713.Oct 25 2019, 20:53

nakihito planned changes to this revision.Oct 25 2019, 22:18

Removed changes to comments.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7877: Diff 13715.Oct 25 2019, 22:31

jasonbcox requested changes to this revision.Oct 26 2019, 00:45

jasonbcox added inline comments.