Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qt: Remove unused `adjustedTime` parameter #14554

Merged
merged 1 commit into from Nov 5, 2018

Conversation

Projects
None yet
8 participants
@hebasto
Copy link
Member

commented Oct 23, 2018

After merging #13622 the adjustedTime parameter in the updateStatus function is unused.

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

utACK cfc55edb6ae7bc49b4df38954fc434ffa831b78f

Nice find @hebasto! Thanks for cleaning this up.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14556 (qt: Fix bug #13299 by hebasto)
  • #14437 (Refactor: Start to separate wallet from node by ryanofsky)
  • #10973 (Refactor: separate wallet from node by ryanofsky)
  • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 24, 2018

Concept ACK, however it looks like the changes could be more comprehensive.

The remaining uses of adjustedTime in /qt/ are in:

  • FormatTxStatus
  • getWalletTxDetails
  • tryGetTxStatus

Of which, it looks like the usage of adjustedTime in atleast FormatTxStatus is also redundant?

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2018

@practicalswift, @fanquake Thank you for your reviews.

@fanquake's comment has been addressed. Please re-review.

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 28, 2018

utACK 58fbed1

Can you squash 58fbed1 into cfc55ed, and prefix the commit message with qt:, so that it's clear that these are qt/wallet changes.

Remove unused `adjustedTime` parameter
qt: After merging #13622 the `adjustedTime` is not used any more in
wallet related functions.

@hebasto hebasto force-pushed the hebasto:20181023-unused-adjustedtime branch to 04972fe Oct 28, 2018

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2018

@fanquake Done.

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 28, 2018

re-utACK 04972fe

@ken2812221
Copy link
Member

left a comment

utACK 04972fe

@fanquake fanquake added this to Mergeable in High-priority for review Nov 2, 2018

@promag

This comment has been minimized.

Copy link
Member

commented Nov 4, 2018

utACK 04972fe.

@laanwj laanwj merged commit 04972fe into bitcoin:master Nov 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Nov 5, 2018

Merge #14554: qt: Remove unused `adjustedTime` parameter
04972fe Remove unused `adjustedTime` parameter (Hennadii Stepanov)

Pull request description:

  After merging #13622 the `adjustedTime` parameter in the `updateStatus` function is unused.

Tree-SHA512: 1d0e03e7343f076ee0032fb721f8ba50571d579958001aab372a43e45b4de24c2bf3bd18c245071cbd69f61ef38182e19666c6f936d55c9085b73c848ba62626

@fanquake fanquake removed this from Mergeable in High-priority for review Nov 5, 2018

@hebasto hebasto deleted the hebasto:20181023-unused-adjustedtime branch Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.