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

Removed goto statements #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

abijith-kp
Copy link

Is there any specific reason to keep the goto statements in the code?
I have tried to remove those statements with minimal changes.

@hoytech
Copy link
Owner

hoytech commented Jul 10, 2016

Thanks for the patch. You're right there's no great reason to prefer goto here since both these instances are essentially loops. However, IMO it's somewhat nice that the destinations have readable labels such as retry_open and bail, and these are removed in this patch.

I'll consider this for the next release, thanks!

@abijith-kp
Copy link
Author

Thank you.
On Jul 10, 2016 7:45 PM, "Doug Hoyte" [email protected] wrote:

Thanks for the patch. You're right there's no great reason to prefer goto
here since both these instances are essentially loops. However, IMO it's
somewhat nice that the destinations have readable labels such as retry_open
and bail, and these are removed in this patch.

I'll consider this for the next release, thanks!


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#36 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ACEcbzLc2z-ILlmvX41eW3shxTP0NV2cks5qUP5zgaJpZM4JDvnQ
.

@hoytech
Copy link
Owner

hoytech commented Aug 8, 2016

Oops, I can't merge this patch because of conflicts caused in another branch I merged in. I'm going to leave this out for the next release, maybe we can get to it in some code clean-up I'm planning later.

@abijith-kp
Copy link
Author

Yeah sure.

On Tue, Aug 9, 2016 at 12:44 AM, Doug Hoyte [email protected]
wrote:

Oops, I can't merge this patch because of conflicts caused in another
branch I merged in. I'm going to leave this out for the next release, maybe
we can get to it in some code clean-up I'm planning alter.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#36 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACEcbzZapdBNA1ACKU3NJlDnYeVIOoJeks5qd4AWgaJpZM4JDvnQ
.

Abijith KP
github.com/abijith-kp
abijithkp.me

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.

2 participants