Page MenuHomePhabricator

Update OSX MIME type for .bitcoinpaymentrequest files, rename identifiers
ClosedPublic

Authored by freetrader on Aug 30 2017, 10:00.

Details

Summary

The OSX property list MIME type application/x-bitcoin-payment-request
is renamed to application/bitcoincash-paymentrequest in line with previous
changes in the wallet. This type is associated with .bitcoinpaymentrequest
files.

The identifiers used in the property list and payment protocol definition
file are updated to 'bitcoincash' to differentiate, and a few comments are
modified to reflect Bitcoin Cash.

No extensions of the existing payment protocol seem to have been
registered at the indicated Wiki URL, so the registration point has
been made into a TODO pending a decision on where such registrations
could occur.

This issue is marked as WIP pending review and confirmation of successful
tests.

Test Plan
make check (on wallet build)
../qa/pull-tester/rpc-tests.py -extended

Gitian builds
Travis builds

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptAug 30 2017, 10:00

Gitian builds and the extended tests worked alright.
Travis testing seems blocked by a hanging test on HEAD at the moment - I'll push this on a branch off the last passing commit to test.

freetrader retitled this revision from [WIP] Update OSX MIME type for .bitcoinpaymentrequest files, rename identifiers to Update OSX MIME type for .bitcoinpaymentrequest files, rename identifiers.Aug 30 2017, 13:35

A few comments. Otherwise looks good to me.

share/qt/Info.plist.in
32 ↗(On Diff #1274)

FYI this reminds me -- currently we build "Bitcoin-Qt.app" on the mac, with the executable being called Bitcoin-Qt.

I think we should rename the Mac app to BitcoinABC-Qt.

(and of course modify this Info.plist.in accordingly).

Having it have the same name as Bitcoin-Qt (core) will be annoying as fuck to users. I run a Mac and I know it would annoy me.

Because that means I can't install both in the standard place on Macs: /Applications.

We don't even have to change much -- just the app bundle name (".app") that gets produced.

I can help you do that -- if you don't feel like worrying about it. I have a mac and can quickly test this.

Let me know!

52 ↗(On Diff #1274)

url scheme should probably be

bitoincash

in line with the other changes in src/qt/paymentserver.cpp, etc.

I think you probably just missed this one. :)

share/qt/Info.plist.in
32 ↗(On Diff #1274)

I looked at this name and saw it in other areas, so would rather do this as a separate change.

I agree with you we should get ABC in there.

52 ↗(On Diff #1274)

Yes, indeed this one slipped thru - will fix, thanks!

Yeah agreed that can be a separate PR.. err... diff.

I'll go ahead and approve this boldly as bitcoin abc. Hopefully noone objects.

This revision is now accepted and ready to land.Aug 30 2017, 14:09
This revision was automatically updated to reflect the committed changes.