Converts qt and wallet modules to use Amount struct together.
These two modules are fairly intertwined, so converting them
together avoids a significant amount of intermediate patches.
Details
Details
- Reviewers
freetrader deadalnix - Group Reviewers
Restricted Project
make VERBOSE=1 check
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- shammah/amount-wallet-and-qt
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1019 Build 1019: arc lint + arc unit
Event Timeline
| src/qt/bitcoinamountfield.h | ||
|---|---|---|
| 23 ↗ | (On Diff #1555) | Worth looking at what that discussion says and update the comment accordingly. Why did you start using qint64 instead of Amount here ? Is the discussion still relevant ? |
| src/qt/bitcoingui.cpp | ||
| 1028 ↗ | (On Diff #1555) | Remove reference |
| src/qt/bitcoingui.h | ||
| 201 ↗ | (On Diff #1555) | Remove reference. |
| src/qt/bitcoinunits.cpp | ||
| 94 ↗ | (On Diff #1555) | Use C++ style constructor qint64(nIn.GetSatoshis()); |
| src/qt/bitcoinunits.h | ||
| 80 ↗ | (On Diff #1555) | Remove reference |
| 84 ↗ | (On Diff #1555) | Remove reference |
| 88 ↗ | (On Diff #1555) | Remove reference |
| 91 ↗ | (On Diff #1555) | Same question about the qint64 |
| 118 ↗ | (On Diff #1555) | Maybe that should be a static method on Amount instead ? |
| src/qt/coincontroldialog.cpp | ||
| 455 ↗ | (On Diff #1555) | Remove reference |
| src/qt/guiutil.h | ||
| 56 ↗ | (On Diff #1555) | Remove reference |
| src/qt/overviewpage.h | ||
| 45 ↗ | (On Diff #1555) | Remove reference x6 |
| src/qt/paymentserver.cpp | ||
| 565 ↗ | (On Diff #1555) | This comment is not accurate anymore. |
| src/qt/paymentserver.h | ||
| 92 ↗ | (On Diff #1555) | Remove reference |
| src/qt/sendcoinsdialog.h | ||
| 62 ↗ | (On Diff #1555) | Remove reference x6 |
| src/qt/transactionfilterproxy.h | ||
| 41 ↗ | (On Diff #1555) | Remove reference |
| src/qt/transactionrecord.cpp | ||
| 135 ↗ | (On Diff #1555) | Maybe implementing the unary minus op would be worth it ? |
| src/qt/transactionrecord.h | ||
| 111 ↗ | (On Diff #1555) | Remove reference x2 |
| src/qt/walletmodel.h | ||
| 43 ↗ | (On Diff #1555) | Remove reference |
| 268 ↗ | (On Diff #1555) | Remove reference x6 |
| src/qt/walletmodeltransaction.h | ||
| 30 ↗ | (On Diff #1555) | Remove reference |
| src/qt/walletview.h | ||
| 133 ↗ | (On Diff #1555) | Remove reference |
| src/validation.h | ||
| 386 ↗ | (On Diff #1555) | This should probably not be boxed in that diff, please split it. Consensus code change should not be bundled with all the QT changes. |
| src/wallet/test/wallet_tests.cpp | ||
| 40 ↗ | (On Diff #1555) | Remove reference |
Comment Actions
I have a few more changes to make after the latest diff, not quite ready for review yet.
| src/qt/bitcoinamountfield.h | ||
|---|---|---|
| 23 ↗ | (On Diff #1555) | I wanted to ask someone more familiar with QT. The moc files that are generated when using Amount do not work properly. I haven't been able to figure out how to fix them. The comment is still applicable. It's because the auto-generated code does not include amount.h. |
| src/qt/bitcoinunits.cpp | ||
| 94 ↗ | (On Diff #1555) | Sounds good to me. Just to clarify: We're okay with including stylistic changes in patches? Normally I've been required to do stylistic changes as part of a larger patch to change things everywhere at once. |
| src/qt/bitcoinunits.h | ||
| 118 ↗ | (On Diff #1555) | Yeah, this was a bit weird. There's already a max money around. I can move it. |
| src/qt/transactionrecord.cpp | ||
|---|---|---|
| 135 ↗ | (On Diff #1555) | I think there's only about 3 uses in the entire codebase. I also tend to think that negating money shouldn't be a convenient operation. |