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 tautology statement #361

Closed
wants to merge 1 commit into from
Closed

Remove tautology statement #361

wants to merge 1 commit into from

Conversation

wn
Copy link

@wn wn commented Jul 15, 2020

Remove tautology statement. Consider the for-loop above, there is no way to break out of the loop unless err != nil.

@puellanivis
Copy link
Collaborator

Looks pretty legit. At the very least, the value of err cannot change in the loop, and as a loop invariant should be tested outside of the loop entirely. But then, as noted, the only way out of the loop above, is if err != nil.

@wn
Copy link
Author

wn commented Jul 18, 2020

Could i get an LGTM?

@puellanivis
Copy link
Collaborator

I just spotted a bug, not in your code, but in the code above it. I’ll fix that, and then push the two together.

@puellanivis
Copy link
Collaborator

Closing with deference to #363 which fixes a bunch of bugs spotted while reviewing this PR. (They’re not bugs from this PR.)

@eikenb
Copy link
Member

eikenb commented Jul 19, 2020

Thanks for handling this @puellanivis!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants