Skip to content

Including csv External sorting using apache commons #23

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

Merged
merged 1 commit into from
Sep 30, 2019

Conversation

guilhermefaleixo
Copy link

Including new class to deal with CsvExternal sorting using apache commons csv

I made small changes into your logic to be able to work with csv files better.

Including new class to deal with CsvExternal sorting using apache commons csv
@lgtm-com
Copy link

lgtm-com bot commented Sep 30, 2019

This pull request introduces 2 alerts when merging 87ed0bc into 8edf467 - view on LGTM.com

new alerts:

  • 1 for Potential output resource leak
  • 1 for Potential input resource leak

@lemire
Copy link
Owner

lemire commented Sep 30, 2019

@guilhermefaleixo Can you review the warnings at LGTM

https://lgtm.com/projects/g/lemire/externalsortinginjava/rev/pr-a4593dbe5b747ee2d5eb5c237f9d3bef6834ac86

And make sure that they are not a concern?

@guilhermefaleixo
Copy link
Author

Hello @lemire the LGTM is down right now, but one of the issues is the inputStream that is not closed but it is the same code that you have in that part and the second issue as far as I am concern it is not an issue as it will be closed anyway when the parser is closed.

@lemire
Copy link
Owner

lemire commented Sep 30, 2019

Ok. Let us merge.

@lemire lemire merged commit a65e2fb into lemire:master Sep 30, 2019
@guilhermefaleixo
Copy link
Author

Thank you so much for your great work @lemire , your library helped me a lot and I hope that this CSV version of it can help as much as helped me =D

@lemire
Copy link
Owner

lemire commented Oct 1, 2019

Thanks for your great contribution.

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.

2 participants