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
Branch
herald-move-miners-to-other-file
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23784
Build 47178: Build Diffecash-herald-tests
Build 47177: arc lint + arc unit

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

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.js
config.js
26 lines
src/
31 lines
test/
mocks/
14 lines
4 lines

Diff 40412

View Options

apps/ecash-herald/config.js

Loading...
View Options

apps/ecash-herald/constants.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...