refactor: rename Wallet field, add alias#373
refactor: rename Wallet field, add alias#373110CodingP wants to merge 2 commits intobitcoindevkit:masterfrom
Wallet field, add alias#373Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #373 +/- ##
==========================================
+ Coverage 85.33% 86.33% +0.99%
==========================================
Files 24 24
Lines 8335 8342 +7
==========================================
+ Hits 7113 7202 +89
+ Misses 1222 1140 -82
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@110CodingP the commit should have an |
thunderbiscuit
left a comment
There was a problem hiding this comment.
Technically it's a
indexed_tx_graph. I'm guessing the shorter name is just here for brevity?
Yes I think we had chosen it for brievity, but honestly the full name works too. More important IMO is that we add the tx part, which is the heart of what the field is (it's not just a graph, it's a transaction graph).
But I also think that because the only transaction graph on the wallet is an indexed one, it's a bit redundant to call it as such, and does make intuitive sense to just call it the wallet's "transaction graph". Just semantics and bikeshedding at this point haha, so don't take my opinion too strongly.
Also I know some people consider it standard to name fields after the full type they hold, like let wallet: Wallet, so under this approach, indexed_tx_graph: IndexedTxGraph would be maybe more "correct". I just find it too much 🤣.
The type alias however... I'm not sure if it's worth the trouble. It's literally used in 1 place, so mostly just a 1-liner redirection (or misdirection?). Using the true type might be cleaner for readers/review.
Description
Renamed
Wallet::indexed_graphtoWallet::tx_graphas the latter is a better name representing the field. Also introduced a type aliasKeychainTxGraphfor convenience.Notes to the reviewers
This PR helps with the multi-keychain work being done in #318 and #367 .
Changelog notice
Checklists
All Submissions:
just pbefore pushing