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

Adapt to Bankscrap 2 #1

Merged
merged 2 commits into from
Apr 29, 2017
Merged

Adapt to Bankscrap 2 #1

merged 2 commits into from
Apr 29, 2017

Conversation

zenitraM
Copy link
Contributor

This library wasn't working properly with the latest Bankscrap, so I did some changes to fix it:

  • 🤑 Money objects with currency returned everywhere
  • 🏦 Connection object unified with Bank and little changes to adapt to Bankscrap 2 interfaces
  • 👮 Some Rubocop warnings fixed
  • 👀 README updated to current Bankscrap CLI

@raulmarcosl
Copy link
Member

Thanks for the PR! None of us have an Openbank account, so it has been difficult to maintain this adapter.

I have reviewed the code and it looks fantastic, congrats! If you say it works is ready to merge. Only one small thing: I saw two TODOs regarding the lack of operation_date in transactions, so I have added them in bankscrap gem. I just released the 2.0.4 version that supports both effective_date and operation_date, so it would be great to use them 🙂

@zjuanma
Copy link
Collaborator

zjuanma commented Apr 16, 2017

I had this plugin updated to bankscrap 2 in my local master branch , tomorrow I push it to origin.
I had Santander plugin updated too.

@raulmarcosl
Copy link
Member

@zjuanma Please, don't push directly to origin master, you are going to generate a lot of conflicts with this PR. Instead, it would be better to have a different PR with your changes to review the code and see the differences with @zenitraM's code, and we can discuss which one merge.

@javiercr
Copy link
Member

@zenitraM and @zjuanma thank you very much for this!

I've reviewed both PRs and they seem to have similar changes. @raulmarcosl and I we've decided to merge this because it includes changes to the readme and a few interesting comments (like this), but in any case @zjuanma kudos to you for working on this one too 🎉!

I'm gonna publish it to rubygems too.

Are you guys planning to maintain this gem? We got a slack channel and it would be nice to have you around to discuss future work on Bankscrap.

@javiercr javiercr merged commit fbb1990 into bankscrap:master Apr 29, 2017
@javiercr
Copy link
Member

Here it is:
https://rubygems.org/gems/bankscrap-openbank

If you have a rubygems account give me your email address and I'll ad you as owners.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants