-
Notifications
You must be signed in to change notification settings - Fork 22
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
Remove all package-level flags #175
Conversation
I have run the test suite in I updated
and then ran the tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
In favor of this overall. We want vitess to be a library, it shouldn't be defining application flags like this.
I need to do a little more digging to see if this is the right amount of changes before I merge this in.
Merging in, I have additional deletions I'll make a separate PR for. |
Summary
This is my attempt to perform a minimally invasive purge of all package-level flags.
I would appreciate some input on:
AuthServerClientCert
it may be better to remove it entirely rather than just prune the flags)Motivation
#174
Testing
I've removed all package-level flags:
The existing tests still pass: