Skip to content
This repository was archived by the owner on Mar 8, 2024. It is now read-only.

Conversation

@dino-rodriguez
Copy link
Member

High Level Overview of Change

Updates public API response to intelligently decide on ACH vs. FIAT
depending on whether the request is a 1.0 or 1.1 response.

Rolls forward the PayID protocol server version to 1.1.

Context of Change

Makes ACH -> FIAT addressDetailsType change non-breaking.

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

We know do some backwards compatibility stuff.

Test Plan

I fixed all existing tests, but need to add more test coverage now in a
subsequent PR.

@nkramer44
Copy link
Collaborator

nkramer44 commented Aug 4, 2020

v1.1.0 uses AchAddressDetails right? If so, I think it should be "if not v1.0.0 or v1.1.0, send FiatAddressDetails, otherwise send AchAddressDetails". Could be wrong, but I think the way you have it squeezes the change into v1.1.0, making v1.1.0 backwards compatible with v1.0.0, not v1.2 backwards compatible with v1.1.0.

Copy link
Collaborator

@nkramer44 nkramer44 left a comment

Choose a reason for hiding this comment

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

Disregard my last comment. All these different versions are tough to keep straight.

Copy link
Collaborator

@0xASK 0xASK left a comment

Choose a reason for hiding this comment

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

LGTM!

@nkramer44 nkramer44 merged commit eea16ca into master Aug 4, 2020
@nkramer44 nkramer44 deleted the dr-ach-fiat-non-breaking branch August 4, 2020 21:35
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.

4 participants