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.

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

milestone: postTokens() ACH client method for API #3

Merged
merged 9 commits into from
Dec 3, 2019

Conversation

gonber
Copy link

@gonber gonber commented Nov 29, 2019

No description provided.

@tewen
Copy link
Owner

tewen commented Dec 3, 2019

@gonber On first read this looks very good. I need to do some local tests and check out the style, since you did everything in this pr (which is fine, not a problem), but it's a lot to digest.

@tewen tewen added the enhancement New feature or request label Dec 3, 2019
@tewen
Copy link
Owner

tewen commented Dec 3, 2019

@gonber Ran through it and the tests and it looks extremely good. You have a style that is very clean and agreeable to my own preferences.

One thing I noticed is that there are a lot of missing semi-colons after expressions. The eslint is not picking this up, but I do have 2 small requests:

  1. See if you can add the eslint rule to check for this.
  2. Fix the places where it's missing.

As of now, that is all I have found. Beyond that, as soon as we merge, we can complete all the milestones.

@gonber
Copy link
Author

gonber commented Dec 3, 2019

@tewen sorry for the typo in the commit message. But hopefully all is good now

@tewen tewen merged commit 9ee5f3b into tewen:master Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants