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

fix: Mistake on setup http_options for Net::HTTP object when build http #124

Merged
merged 4 commits into from
Jan 7, 2021

Conversation

hoangtuanictvn
Copy link
Contributor

@hoangtuanictvn hoangtuanictvn commented Jan 3, 2021

Fixes

Add http_options when setup Net:HTTP object to fix the mistake from PR #67

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jan 3, 2021
@codecov
Copy link

codecov bot commented Jan 3, 2021

Codecov Report

Merging #124 (4cd4a4d) into main (3059289) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   95.60%   95.75%   +0.14%     
==========================================
  Files           2        2              
  Lines         341      353      +12     
==========================================
+ Hits          326      338      +12     
  Misses         15       15              
Impacted Files Coverage Δ
lib/ruby_http_client.rb 91.66% <100.00%> (+0.43%) ⬆️
test/test_ruby_http_client.rb 97.85% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3059289...4cd4a4d. Read the comment docs.

@hoangtuanictvn
Copy link
Contributor Author

hoangtuanictvn commented Jan 3, 2021

Happy new year!
Because of the old date in the license, the Travis CI had failed. If it is possible, I will update the year of the license from 2020 to 2021.

@hoangtuanictvn hoangtuanictvn changed the title fix: Mistake on setup http_options to Net::HTTP object when build http fix: Mistake on setup http_options for Net::HTTP object when build http Jan 3, 2021
Copy link
Contributor

@eshanholtz eshanholtz left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you so much for the contribution!

@hoangtuanictvn
Copy link
Contributor Author

hoangtuanictvn commented Jan 6, 2021

@eshanholtz
Thank you,
I am sorry, but would it be possible to change the date in the license?

@eshanholtz
Copy link
Contributor

@hoangtuanictvn

Yup! We're looking to get the license updated across the board today.

@hoangtuanictvn
Copy link
Contributor Author

hoangtuanictvn commented Jan 7, 2021

@eshanholtz
I have updated the code based on the update of the license.
Because the old implemented in Net:HTTP on ruby versions 2.4, 2.5 and jruby. CI didn't pass with a small error in the test. But don't worry, I fixed it there 4cd4a4d . And I hope that you can review it again.

Thanks a lot!

Copy link
Contributor

@JenniferMah JenniferMah left a comment

Choose a reason for hiding this comment

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

Hi @hoangtuanictvn everything looks good to me! Thanks for your contribution!

@hoangtuanictvn
Copy link
Contributor Author

hoangtuanictvn commented Jan 10, 2021

Good morning,

I have pushed a PR sendgrid/sendgrid-ruby#455 to add some feature about http_options in the sendgrid-ruby gem. However, the test had failed (sendgrid/sendgrid-ruby#455 (comment)) because this PR hasn't released. I am waiting it. So, could you tell me when is it release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants