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

implement csv table with logs #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickolay-github
Copy link

@nickolay-github nickolay-github commented Nov 19, 2020

I decided to participate in open source, I hope after 4 years this enhancment issue #7 is still relevant :)

I added the output of messages to the csv file which is easy to navigate

Perhaps you meant the table output to the console

If so, please let me know - I will redo

@thomai
Copy link
Owner

thomai commented Dec 23, 2020

Thanks a lot for the pull request!

Could you please update your changes a bit regarding the following change requests?

  • I suggest to have a CLI parameter to provide a file path for the exported CSV file (using argparse)
  • If the CLI parameter is not set, the results should not be exported (I think most of the people do not need such a file)
  • If you want to cover Change console output to table #7, the output needs to be formatted as a table (on the command-line). This issue is not about exporting a CSV file. I am not saying that your idea about exporting a CSV file does not make sense. I really appreciate that feature as well!

@nickolay-github
Copy link
Author

Thanks for the feedback!

I agree with the points, I will refactor the PR based on the comment points

@nickolay-github
Copy link
Author

nickolay-github commented Mar 15, 2021

Completed in another pull request - #22

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