Skip to content

osx: make use of the 10.8+ user notification center to display growl lik...#2615

Merged
laanwj merged 1 commit intobitcoin:masterfrom
jonasschnelli:mac10_8_not_center
May 30, 2013
Merged

osx: make use of the 10.8+ user notification center to display growl lik...#2615
laanwj merged 1 commit intobitcoin:masterfrom
jonasschnelli:mac10_8_not_center

Conversation

@jonasschnelli
Copy link
Copy Markdown
Contributor

...e notifications

  • if 10.8, use user notification center, if <10.8, use growl

why:

  • Growl is since 2.0 no longer free available (needs to be purchased for ~2$) and i think bitcoin should have at least a 2nd way for users who want to use free software or a free alternative (notification center is includes in 10.8+)

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

unbenannt-2
that's how it looks.

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

It would also be possible to add a settings flag where the user can switch between growl and 10.8+ user notification center.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented May 3, 2013

Don't add an option for trivial stuff such as this, if you think this looks better or is more useful (as you say, it's integrated and free), go for it.

@gavinandresen
Copy link
Copy Markdown
Contributor

I vaguely recall us having issues with growl notifications in the past. Did you test this on OSX 10.5 ?

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

@gavinandresen No. It's not yet tested on 10.5 to 10.7. I'll do that in the next days. I also fix the pull-tester-report issue.

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

@gavinandresen getting crazy while building on 10.6/32bit. :)
After a proper qmake/make and fancy build (dmg) i try to run Bitcoin-Qt on 10.5 but get:

dyld: Library not loaded: @executable_path/../Frameworks/libssl.1.0.0.dylib
  Referenced from: /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
  Reason: no suitable image found.  Did find:
    /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/../Frameworks/libssl.1.0.0.dylib: unknown required load command 0x80000022
    /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/../Frameworks/libssl.1.0.0.dylib: unknown required load command 0x80000022
Trace/BPT trap

libssl looks okay:
lipo -info Contents/Frameworks/libssl.1.0.0.dylib Architectures in the fat file: Contents/Frameworks/libssl.1.0.0.dylib are: i386 x86_64

any ideas?

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

puh.. had to grab out a old mac mini to do 10.6/32bit builds. Testes on 10.5 (VM) and 10.6. After some changes it worked perfect.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b5c1406fd043bcb7d4587ad9494d1d8bca70c29c for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@Diapolo
Copy link
Copy Markdown

Diapolo commented May 11, 2013

Is this also Qt5 compatible :)?

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

@Diapolo yes. I initially wrote this patch for Qt5 compatibility because it can handle growl without qt platform functions.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented May 19, 2013

ACK (on code changes, cannot test Mac changes)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also use our style here please :).

void MacNotificationHandler::sendAppleScript(const QString &script)
{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. yes. will change it.

…like notifications

- if 10.8, use user notification center, if <10.8, use growl

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>
@jonasschnelli
Copy link
Copy Markdown
Contributor Author

fixed some minor code-style related things.
@fanquake can you do a test on mac and give a final ACK?
other final ACKs?

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b4b017059514ed0157877984363ed94f5ab098e9 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@fanquake
Copy link
Copy Markdown
Member

@jonasschnelli I'll test it this weekend

laanwj added a commit that referenced this pull request May 30, 2013
osx: make use of the 10.8+ user notification center to display growl lik...
@laanwj laanwj merged commit a2d2e5e into bitcoin:master May 30, 2013
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants