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

remove dead code for checking error nil #282

Merged
merged 1 commit into from Sep 10, 2020

Conversation

yashbhutwala
Copy link

Here is the previous dead code:

  1. Assigning: "err" = "nil".
  2. At condition "err != nil", the value of "err" must be "nil".
  3. The condition "err != nil" cannot be true.

@CLAassistant
Copy link

CLAassistant commented Sep 10, 2020

CLA assistant check
All committers have signed the CLA.

@cornfeedhobo
Copy link

We should look through git history to see what the reasoning is for this happening

@yashbhutwala
Copy link
Author

This looks like the offending commit: b22fc70#diff-908f4c077f45928d663a0b84f166e7bcR96 via git blame: https://github.com/spf13/pflag/blame/master/string_array.go#L36

@yashbhutwala
Copy link
Author

doesn't look like there was much review for that PR: #216, so seems like a honest mistake + lack of review process.

@cornfeedhobo
Copy link

@yashbhutwala Ahh yes, I see. This looks like a mistake from a copy-pasta event. Unless you protest, I'd like to apply this on my fork later (or you can submit an equivalent PR). Thanks!!

@yashbhutwala
Copy link
Author

huh? why do you maintain your own fork? Is this project not community maintained? If not, maybe there should be a note in the top level README. @spf13?

@eparis eparis merged commit 6971c29 into spf13:master Sep 10, 2020
@cornfeedhobo
Copy link

cornfeedhobo commented Sep 10, 2020

@yashbhutwala it is haphazardly maintained. I've tried to probe the maintainers on various issues and get varying levels of responses (you can look at my PR for a test that has been broken for 2+ years now). I'm just maintaining a fork now :)

Edit: This is kind of the history of pflag. They forked it from a person that stopped maintaining it, and I'm doing that now.

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

4 participants