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

extend stringToString pflag binding to stringToInt pflag #1435

Closed
wants to merge 3 commits into from

Conversation

vorishirne
Copy link
Contributor

Support for pflag's StringToString was added long back, but Viper Still couldn't bind to pflag StringToInt. Addid that support.

@CLAassistant
Copy link

CLAassistant commented Sep 24, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

👋 Thanks for contributing to Viper! You are awesome! 🎉

A maintainer will take a look at your pull request shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news,
either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

@vorishirne
Copy link
Contributor Author

@sagikazarmark can u look into this

Copy link
Collaborator

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sending a PR! Can you please take a look at the comment I left?

viper.go Outdated
@@ -1373,7 +1373,7 @@ func readAsCSV(val string) ([]string, error) {

// mostly copied from pflag's implementation of this operation here https://github.com/spf13/pflag/blob/master/string_to_string.go#L79
// alterations are: errors are swallowed, map[string]interface{} is returned in order to enable cast.ToStringMap
func stringToStringConv(val string) interface{} {
func csvKeyValueConv(val string) interface{} {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't stringToInterfaceCong be a better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure let me do that

Copy link
Contributor Author

@vorishirne vorishirne Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. But moved to another MR. Some issue happened in this MR

New MR: #1462

@vorishirne vorishirne deleted the branch spf13:master November 8, 2022 12:30
@vorishirne vorishirne closed this Nov 8, 2022
@vorishirne vorishirne deleted the master branch November 8, 2022 12:30
@vorishirne
Copy link
Contributor Author

@sagikazarmark I mistakenly closed this MR. I have created a new MR

#1462

Your suggestion has been implemented there.

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.

None yet

3 participants