Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

WIP: Chunk Pay id demos #50

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

rmurphy23
Copy link

High Level Overview of Change

  • Chunking the Java PayID demo in 3 separate files one resolving on the BTC Network, on resolving on XRPL, and one resolving all PayID's belonging to Alice.
  • Chunking the NodeJS PayID demo in 3 separate files one resolving on the BTC Network, on resolving on XRPL, and one resolving all PayID's belonging to Alice.
  • Chunking the Swift PayID demo in 3 separate files one resolving on the BTC Network, on resolving on XRPL, and one resolving all PayID's belonging to Alice.

Context of Change

This change was a simple refactor. It was necessary because if a user going through this demo did not not have a PayID associated with both the XRPL and BTC networks the demo would result in a fatal error. This refactor allows users to demo the PayID resolve according to the networks for which their PayID is associated.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [**] Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Before / After

Before the refactor there was one file for each of the PayID demos according to their respective language NodeJS, Java, and Swift. After there is three files for each PayID demo according to their respective languages.

Test Plan

Using VS Code I was able to test the NodeJS refactored demos and Java (Maven) refactored demos which both resulted in expected output. I've been unable to test the refactored Swift demos thus they will need more attention.

@amiecorso can you review this?

Copy link
Contributor

@amiecorso amiecorso left a comment

Choose a reason for hiding this comment

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

This is looking good Ryan. I just fixed some cosmetic stuff - I know this is probably just stuff you retained from the original demos, but I figure now is a good time to clean it up a little.

public static void main(String[] args) throws PayIdException {
// The Pay ID to resolve.
String payId = "alice$dev.payid.xpring.money";

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

// The BTC network to resolve on.
String btcNetwork = "btc-testnet";

// A client to resolve PayIDs on any network..
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// A client to resolve PayIDs on any network..
// A client to resolve PayIDs on any network.


// The XRP Ledger network to resolve on.
XrplNetwork xrpNetwork = XrplNetwork.MAIN;

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

// The PayID to resolve.
const payId = "alice$dev.payid.xpring.money";

// A client to resolve PayIDs on the Bitcoin testnet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// A client to resolve PayIDs on the Bitcoin testnet.
// A client to resolve PayIDs on any network.

// The BTC network to resolve on.
const btcNetwork = "btc-testnet";

// A client to resolve PayIDs on the XRP Ledger.
const xrpPayIdClient = new XrpPayIdClient(xrpNetwork);

// A client to resolve PayIDs on the Bitcoin testnet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// A client to resolve PayIDs on the Bitcoin testnet.
// A client to resolve PayIDs on any network.

import Foundation
import XpringKit

// The Pay ID to resolve.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// The Pay ID to resolve.
// The PayID to resolve.

// The Pay ID to resolve.
let payID = "alice$dev.payid.xpring.money"

// A client to resolve PayIDs on any network..
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// A client to resolve PayIDs on any network..
// A client to resolve PayIDs on any network.

// A client to resolve PayIDs on any network..
let payIDClient = PayIDClient()

// Resolve all addresses
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Resolve all addresses
// Resolve all addresses.

// Resolve on Bitcoin testnet.
let btcNetwork = "btc-testnet"

// A client to resolve PayIDs on any network..
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// A client to resolve PayIDs on any network..
// A client to resolve PayIDs on any network.

import Foundation
import XpringKit

// The Pay ID to resolve.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// The Pay ID to resolve.
// The PayID to resolve.

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.

2 participants