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

Get more details about the applied fix #86

Merged
merged 4 commits into from
Jul 28, 2017

Conversation

jose
Copy link
Collaborator

@jose jose commented May 29, 2017

No description provided.

@rjust
Copy link
Owner

rjust commented Jul 19, 2017

Hi @jose,

Thanks for these improvements and the test cases!

There is one possible drawback of the current solution, which is that the logging in the fix_test_suite script now uses the DB module. This means that the user is required to install DBI and the corresponding driver for CSV files. This is fine and consistent with the other analysis scripts, but maybe we should make the verbose logging optional so that the fix_test_suite script does not depend on the DB module by default. Any thoughts?

Thanks,
René

@jose
Copy link
Collaborator Author

jose commented Jul 19, 2017

I'm ok with that. So, should we add a new option to the script? And, if enabled, fix_test_suite uses DB, otherwise (and by default) it does not use it.

@jose jose force-pushed the numbers_from_fix_test_suite branch from b112170 to 8996ea3 Compare July 27, 2017 02:14
@jose jose force-pushed the numbers_from_fix_test_suite branch from 8996ea3 to ad19182 Compare July 27, 2017 02:17
@jose
Copy link
Collaborator Author

jose commented Jul 27, 2017

@rjust, just to let you know that, by default, fix_test_suite.pl script does not require DB module anymore.

@rjust rjust merged commit cb80241 into rjust:master Jul 28, 2017
@jose jose deleted the numbers_from_fix_test_suite branch February 17, 2022 14:24
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