Backporting univalue. Also fix broken test data from previous commit.
ClosedPublic
Actions

Authored by jasonbcox on Dec 2 2017, 02:04.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGINGe2c428c5b1fb: Backporting univalue. Also fix broken test data from previous commit.
rABCe2c428c5b1fb: Backporting univalue. Also fix broken test data from previous commit.

Summary

Backport latest univalue version to fix minor bugs and cleanup namespaces. This change also fixes the test data that broke from the previously reverted change.

Test Plan

Tests pass (now).

Diff Detail

Repository

rABC Bitcoin ABC

Branch

master

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1232
Build 1232: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Dec 2 2017, 02:04

Harbormaster completed remote builds in B1211: Diff 1888.Dec 2 2017, 02:04

Owners added a reviewer: Restricted Owners Package.Dec 2 2017, 02:04

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 2 2017, 02:04

Herald added a reviewer: Restricted Project. · View Herald Transcript

deadalnix requested changes to this revision.Dec 3 2017, 15:38

deadalnix added inline comments.

.arclint
7 ↗	(On Diff #1888)	Revert.
src/rpc/server.cpp
29 ↗	(On Diff #1888)	This has nothing to do with updating univalue.

This revision now requires changes to proceed.Dec 3 2017, 15:38

Reverted change to .arclint

jasonbcox marked 2 inline comments as done.Dec 3 2017, 18:49

jasonbcox added inline comments.

src/rpc/server.cpp
29 ↗	(On Diff #1888)	I removed it as part of the initial task, which was to remove namespace usage. The univalue change was also intended to do the same, as there were a few imported namespaces in the univalue code.

deadalnix requested changes to this revision.Dec 3 2017, 19:32

deadalnix added inline comments.

src/rpc/server.cpp
29 ↗	(On Diff #1888)	Please keep diff into logically separated units. Upgrading univalue is one. Remove this is another. If it turns out there is a problem with removing this import, do you think we should also revert everything related to univalue ? Or the ther way around, if upgrading univalue has some side effect, should we reintroduce this as well ? Nope, these are two different things. They belong in two separate diffs.

This revision now requires changes to proceed.Dec 3 2017, 19:32

jasonbcox added inline comments.Dec 4 2017, 04:25

src/rpc/server.cpp
29 ↗	(On Diff #1888)	Makes sense. I'll split them apart.

Split this review into two diffs.

jasonbcox added inline comments.Dec 4 2017, 07:43

src/rpc/server.cpp
29 ↗	(On Diff #1888)	Split into a separate diff: https://reviews.bitcoinabc.org/D749 and removed this change from this diff.

jasonbcox marked an inline comment as done.Dec 4 2017, 07:43

jasonbcox retitled this revision from Get rid of namespace imports & backporting univalue. Also fix broken test data from previous commit. to Backporting univalue. Also fix broken test data from previous commit..

jasonbcox edited the summary of this revision. (Show Details)

deadalnix accepted this revision.Dec 4 2017, 11:07

This revision is now accepted and ready to land.Dec 4 2017, 11:07

Closed by commit rABCe2c428c5b1fb: Backporting univalue. Also fix broken test data from previous commit. (authored by jasonbcox). · Explain WhyDec 5 2017, 01:56

This revision was automatically updated to reflect the committed changes.