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 what appeared to be a duplicate code block in docs #23

Merged
merged 1 commit into from
Apr 4, 2020

Conversation

rubenflamshepherd
Copy link
Contributor

Was reading the docs and noticed. Please ignore if this is overly unsolicited.

@CLAassistant
Copy link

CLAassistant commented Apr 3, 2020

CLA assistant check
All committers have signed the CLA.

@rubenflamshepherd rubenflamshepherd changed the title Removed what appeared to be a duplicate code block Removed what appeared to be a duplicate code block in docs Apr 3, 2020
@huan
Copy link
Owner

huan commented Apr 3, 2020

Thanks for your PR for trying to fix this!

This code was added 3 years ago because I found that the GitHub web service is not stable enough.

At that time, the request to https://raw.githubusercontent.com/huan/gasl/master/src/gas-log-lib.js sometimes will fail, and it will succeed after retry one more time.

I believe GitHub service should be more stable today because the time has passed a lot and it gets more resources from Microsoft. So I'm ok with this PR to remove the duplicated code.

Please let me know what you think. If you think this code is ok to be removed after reading this, I'll merge this PR.

@rubenflamshepherd
Copy link
Contributor Author

Hey huan!

I came across gast in this article here where they use only one call to eval. I have also been only using the single call and haven't run into any issues. I do think github should be stable enough now to remove the duplicate call.

@huan huan merged commit 42a9c01 into huan:master Apr 4, 2020
@huan
Copy link
Owner

huan commented Apr 4, 2020

Thanks for letting me know the article and your experience, the PR has been merged.

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.

3 participants