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

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
Member

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 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 portfolio-performance:master Oct 29, 2016
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

2 participants