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

Improvement of Comdirect parser #640

Merged
merged 2 commits into from Oct 29, 2016
Merged

Improvement of Comdirect parser #640

merged 2 commits into from Oct 29, 2016

Conversation

@bbrach
Copy link
Contributor

@bbrach bbrach commented Oct 21, 2016

Fee is added correctly, when there is no seperate sum in the invoice
(e.g. LiveTrading and saving plan)

Fee is added correctly, when there is no seperate sum in the invoice
(e.g. LiveTrading and saving plan)
@buchen
Copy link
Owner

@buchen buchen commented Oct 22, 2016

Hi @bbrach,

vielen Dank für die Contribution! Da ich selber nur bei einer Bank bin, ist das besonders hilfreich wenn andere zu den PDF Parsing beitragen.

Mir sind ein paar Kleinigkeiten aufgefallen:

Erstens hast Du in Deinem Commit als Email Adresse bbrach AT users DOT noreply DOT github DOT com drin stehen. Wenn das Absicht ist das okay. Die Email Adresse sah nach einem "Default" aus.

Zweitens hast ist die Beispieldatei "Kauf3" dabei, aber anscheinend wird die in den Tests gar nicht genutzt. Zumindest scheint es keine Change in der Datei ComdirectPDFExtractorTest zu sein. Fehlt das vielleicht noch der Commit?

Ansonsten sieht das gut aus.

@bbrach
Copy link
Contributor Author

@bbrach bbrach commented Oct 23, 2016

Hallo,

  1. Das ist eine private von Github zur Verfügung gestellte E-Mail, weil ich meine E-Mail Adresse ungern wegen Spam öffentlich verwenden möchte. Siehe auch: https://help.github.com/articles/keeping-your-email-address-private/
  2. Ok ich habe es nachgepflegt
@buchen buchen merged commit 215d335 into buchen:master Oct 29, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.