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

@alloy => Manual CC Entry Problem 📱 #475

Merged
merged 5 commits into from Jun 29, 2015
Merged

@alloy => Manual CC Entry Problem 📱 #475

merged 5 commits into from Jun 29, 2015

Conversation

ashfurrow
Copy link
Contributor

Crus is that we weren't sending a "complete" event to our finishedSignal once the CC tokenization succeeded. I added tests to ensure that the event is sent if and only if the tokenization succeeds.

I also noticed a few other problems that I fixed and added test for, in separate commits. Should I make things like that separate PRs? Open to advice.

(Note: Circle and Travis are currently both failing to run tests. I've checked that they pass locally.)

Fixes #455.

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

Tested for real/device?

@ashfurrow
Copy link
Contributor Author

I've not tested it on a device yet; will do after lunch.

@ashfurrow
Copy link
Contributor Author

(Thanks for the reminder!)

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

Alrighty then, I’ll wait to hear when you tested it on device.

Regarding splitting off PRs, I’m ok with this personally. If you feel wrong and it would keep you up at night you could just do the fix and then a new PR with all the ‘UI’ issues you came across.

@ashfurrow
Copy link
Contributor Author

Separate PRs are hard since sometimes the fixes I do depend on the other code I've already changed for the main issue. Gonna keep it like this, in small commits 👍

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

@ashfurrow To be clear, I meant that I’m personally ok with not splitting off, i.e. as-is.

@ashfurrow ashfurrow changed the title @alloy => Manual CC Entry Problem @alloy => Manual CC Entry Problem 📱 Jun 29, 2015
@ashfurrow
Copy link
Contributor Author

@alloy verified on device.

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

💯 👍

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

Oh, forgot to notice before, but CHANGELOG?

@ashfurrow
Copy link
Contributor Author

Knew there was something I was forgetting.

@alloy
Copy link
Contributor

alloy commented Jun 29, 2015

💯 +1 👍

alloy added a commit that referenced this pull request Jun 29, 2015
@alloy => Manual CC Entry Problem 📱
@alloy alloy merged commit 8920fdb into master Jun 29, 2015
@alloy alloy deleted the cc-entry branch June 29, 2015 18:49
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.

Swallowed error on entering cc
2 participants