Conversation
mrigger
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR. The changes look great overall.
I think that we need to expose the default port and host to the user in some way. See my thoughts in this respect as an in-line comment. Let me know if you have any follow-up questions or thoughts.
Thanks again for the great PR!
Sorry, something went wrong.
|
TBR. Please let me know if this version can be merged? Thank you @mrigger |
Sorry, something went wrong.
|
Sorry for the delayed response! As mentioned, it would be great to address the two concerns I mentioned: (1) we want to display the default values that each DBMS uses to the user as part of the help text and (2) not use invalid default values (i.e., either use |
Sorry, something went wrong.
|
TBR @mrigger |
Sorry, something went wrong.
Thanks a lot again for your changes and for your patience! After introducing constants and using them to replace the hard-coded values, we can likely merge the PR. |
Sorry, something went wrong.
|
TBR @mrigger Thanks for your suggestions. Please review the latest revisions, and if necessary, please suggest revisions. |
Sorry, something went wrong.
|
Thanks a lot for your PR and for incorporating my improvement suggestions. I highly appreciate it! Merging the PR now. For future PRs, please pay attention to a "clean" commit history by avoid merges as well as rewriting the history for incorporating feedback (and removing accidentally committed files), see https://github.com/sqlancer/sqlancer/blob/master/CONTRIBUTING.md#commit-history. |
Sorry, something went wrong.
Thanks for your suggestions. It’s a great honor to contribute to this project. |
Sorry, something went wrong.
What problem does this PR solve?
Issue number: #95
Summary
Supporting host/port arguments for the follow databases:
Release note
No release note