-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Rename configparser.Parser
as config.Map
#4075
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
Conversation
4f9c2b7
to
577aead
Compare
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.
configmap.ConfigMap stutters, but I don't have a better alternative to offer.
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.
configmap.ConfigMap stutters, but I don't have a better alternative to offer.
I agree with this, the only way I could imagine this not sounding repetitive is by moving out of the config
package altogether. Moving it to config
makes it slightly better to use config.ConfigMap
but I don't know if it's worth the effort.
6ab7780
to
4a66a6d
Compare
@codeboten @dashpole tried to make this |
configparser.Parser
as config.Map
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.
Nice. This is actually much easier to read, and not just because I don't confuse it with k8s configmaps anymore.
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.
I agree w/ @dashpole, this is great! Just a couple of nits in the comments
Signed-off-by: Bogdan Drutu <[email protected]>
4559625
to
2a60df9
Compare
Signed-off-by: Bogdan Drutu <[email protected]>
2a60df9
to
910c2d9
Compare
Signed-off-by: Bogdan Drutu [email protected]