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: Notificator class refactoring #15007

Merged
merged 1 commit into from Jan 4, 2019

Conversation

Projects
None yet
6 participants
@hebasto
Copy link
Member

commented Dec 19, 2018

This PR:

  • removes misplaced Q_UNUSED(cls); cls is actually used:

    switch(cls)

  • removes unused parameters in functions notifySystray() and notifyMacUserNotificationCenter()

  • improves comments

Remove misplaced Q_UNUSED and others enhancements
Also this removes unused function parameters and improves comments.

@fanquake fanquake added the GUI label Dec 19, 2018

@practicalswift

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

Concept ACK

Nice cleanup!

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2018

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

Conflicts

No conflicts as of last run.

@promag

This comment has been minimized.

Copy link
Member

commented Dec 31, 2018

Concept ACK, will test.

@promag

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

tACK 698d0f8 on macOS 10.14.1. Updated comments also LGTM.

@fanquake

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

Quickly-tested ACK 698d0f8 Checked that notifications work on macOS:
notifications - 15007

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK 698d0f8

@laanwj laanwj merged commit 698d0f8 into bitcoin:master Jan 4, 2019

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 Jan 4, 2019

Merge #15007: qt: Notificator class refactoring
698d0f8 Remove misplaced Q_UNUSED and others enhancements (Hennadii Stepanov)

Pull request description:

  This PR:
  - removes misplaced `Q_UNUSED(cls)`; `cls` is actually used:
  https://github.com/bitcoin/bitcoin/blob/eb7daf4d600eeb631427c018a984a77a34aca66e/src/qt/notificator.cpp#L188

  - removes unused parameters in functions `notifySystray()` and `notifyMacUserNotificationCenter()`

  - improves comments

Tree-SHA512: 78c0713f2a968b471dae422e9a5a0959018923e0d24ed595921001a9895ffb6ceb0311c63e4264fdff470b021a8b8df0f6972c630a051dafed06281880acc261

@hebasto hebasto deleted the hebasto:20181219-notificator-enhancement branch Jan 4, 2019

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.