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.

Sign up for GitHub

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

Jump to bottom

Add Account owner full name field to Uphold payment method #5500

Merged
merged 5 commits into from
Jun 1, 2021
Merged

Add Account owner full name field to Uphold payment method #5500

merged 5 commits into from
Jun 1, 2021

Conversation

BtcContributor
Copy link
Contributor

Fixes #5493

Now it looks like this:

pazza_uphold_fix3

pazza_uphold_fix

pazza_uphold_fix2

ripcurlx
ripcurlx suggested changes May 18, 2021
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

NACK - Please see my comment regarding proto changes

proto/src/main/proto/pb.proto Outdated Show resolved Hide resolved
@BtcContributor
Copy link
Contributor Author

Restored proto order and add backward compatibility, like done in #5117

(Not sure why screenshots are yellow...)

uphold_back

uphold_back2

Not sure if we should also update existing account at startup. What do you think @pazza83 and @ripcurlx?

@pazza83
Copy link

pazza83 commented May 18, 2021

Looks great thanks for this.

I would be in favor of updating existing accounts at startup if not too much additional work

@BtcContributor BtcContributor requested a review from ripcurlx May 18, 2021 21:32
ripcurlx
ripcurlx suggested changes May 19, 2021
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

NACK - proto and backwrd. comp. are looking fine now, just a couple of minor adjustments needed.

core/src/main/java/bisq/core/payment/UpholdAccount.java Outdated Show resolved Hide resolved
core/src/main/java/bisq/core/payment/UpholdAccount.java Show resolved Hide resolved
core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java Outdated Show resolved Hide resolved
@ripcurlx ripcurlx added this to the v1.6.5 milestone May 19, 2021
ripcurlx
ripcurlx previously approved these changes May 21, 2021
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx modified the milestones: v1.6.5, v1.6.6 May 21, 2021
@BtcContributor BtcContributor requested a review from ripcurlx June 1, 2021 08:05
ripcurlx
ripcurlx approved these changes Jun 1, 2021
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx merged commit 7883df2 into bisq-network:master Jun 1, 2021
@BtcContributor BtcContributor deleted the fix_uphold branch June 1, 2021 11:04
@BtcContributor BtcContributor mentioned this pull request Jun 3, 2021
Closed
@ripcurlx ripcurlx mentioned this pull request Jun 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skarelin skarelin skarelin left review comments

@ripcurlx ripcurlx ripcurlx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7.0
Development

Successfully merging this pull request may close these issues.

Uphold payment method issues to fix
4 participants
@BtcContributor @pazza83 @ripcurlx @skarelin