Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Removed Venmo
fallbacktoWeb
#1434Removed Venmo
fallbacktoWeb
#1434Changes from 17 commits
b92fa87
3384428
1a24560
438dfc7
45f21bc
47a89a5
bf49d4f
3e5842f
73575a0
0d5916b
398c20d
9a6759f
a4e6d8b
c058621
cd51766
50aaa40
e2c7a67
d4af84a
c710d4d
c3225a1
2d0d7e0
b265b69
0d6f01e
cb531ad
5b536c3
ab02597
dae4473
c7b615b
7a19f68
5fb49c3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does verifyAppSwitch need to bother with returning any value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed it would still need to return
true
sinceverifyAppSwitch
checks if Venmo is enabled and if Venmo app is installed. Let me know if you think it's ok to remove the Bool return otherwiseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
verifyAppSwitch
is only being used here I think it should be safe. I think the function will just throw an error if not enabled, and the returned value isn't being used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will want to add back this return, my guess is that this is why these tests are failing:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added it back and tested locally. Seems like removing
return
was the cause of the failed tests