Page MenuHomePhabricator
Differential D13930

[herald] Move non-config constants to separate file
ClosedPublic
Actions

Authored by bytesofman on May 24 2023, 20:09.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCf083ffe042ce: [herald] Move non-config constants to separate file
Summary

T2972

Following feedback from D13875, creating a separate constants.js file to hold constant values that are not related to config.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman created this revision.May 24 2023, 20:09
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 24 2023, 20:09
bytesofman requested review of this revision.May 24 2023, 20:09
bytesofman mentioned this in D13875: [herald] Upgrade miner parsing.May 24 2023, 20:09
Harbormaster completed remote builds in B23784: Diff 40412.May 24 2023, 20:10
Fabien requested changes to this revision.May 25 2023, 12:38
Fabien added a subscriber: Fabien.
Fabien added inline comments.
apps/ecash-herald/constants.js
62 ↗(On Diff #40412)

OK on the concept, but constants.js is not really self-explanatory and the miners are not related to layer apps.
I suggest you put them in separated files in the same constants directory, e.g. constants/miners.js and constants/op_return.js.

This revision now requires changes to proceed.May 25 2023, 12:38
bytesofman updated this revision to Diff 40436.May 25 2023, 16:12

Better folder organizing for constants

Harbormaster completed remote builds in B23800: Diff 40436.May 25 2023, 16:13
Fabien accepted this revision.May 25 2023, 20:05
This revision is now accepted and ready to land.May 25 2023, 20:05
Closed by commit rABCf083ffe042ce: [herald] Move non-config constants to separate file (authored by bytesofman). · Explain WhyMay 25 2023, 23:28
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCf083ffe042ce: [herald] Move non-config constants to separate file.

Revision Contents
Changeset List

PathSize
apps/
ecash-herald/
49 lines
constants/
18 lines
48 lines
src/
32 lines
test/
mocks/
14 lines
4 lines

Diff 40444

View Options

apps/ecash-herald/config.js

Loading...
View Options

apps/ecash-herald/constants/miners.js

Loading...
View Options

apps/ecash-herald/constants/op_return.js

Loading...
View Options

apps/ecash-herald/src/parse.js

Loading...
View Options

apps/ecash-herald/test/mocks/memo.js

Loading...
View Options

apps/ecash-herald/test/parseTests.js

Loading...