Skip to content

Replace usages of http with excon#13800

Merged
JamieMagee merged 2 commits intodependabot:mainfrom
yeikel:techdebt/unify-single-http
Jan 30, 2026
Merged

Replace usages of http with excon#13800
JamieMagee merged 2 commits intodependabot:mainfrom
yeikel:techdebt/unify-single-http

Conversation

@yeikel
Copy link
Copy Markdown
Contributor

@yeikel yeikel commented Dec 16, 2025

What are you trying to accomplish?

Removed the dependency on the http library and unified the http calls behind excon

Closes #10881

Anything you want to highlight for special attention from reviewers?

Sorbet needed a Shim which I documented in the code

How will you know you've accomplished your goal?

  • We no longer need http library as a dependency
  • The existing test pass

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@yeikel yeikel requested a review from a team as a code owner December 16, 2025 18:56
@github-actions github-actions bot added the L: ruby:bundler RubyGems via bundler label Dec 16, 2025
@yeikel yeikel force-pushed the techdebt/unify-single-http branch 5 times, most recently from 31db129 to 6d12771 Compare December 18, 2025 15:10
@yeikel yeikel closed this Dec 18, 2025
@yeikel yeikel reopened this Dec 18, 2025
@yeikel yeikel force-pushed the techdebt/unify-single-http branch 4 times, most recently from b08b43f to da56526 Compare December 23, 2025 02:08
@yeikel yeikel force-pushed the techdebt/unify-single-http branch 6 times, most recently from 91c13bc to 2a6ea8f Compare January 6, 2026 23:58
@yeikel yeikel force-pushed the techdebt/unify-single-http branch from 2a6ea8f to f449633 Compare January 7, 2026 19:17
@yeikel yeikel force-pushed the techdebt/unify-single-http branch from f449633 to 4dc191c Compare January 14, 2026 20:21
@yeikel
Copy link
Copy Markdown
Contributor Author

yeikel commented Jan 14, 2026

@JamieMagee Any chance you can review this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@yeikel This seems ok, but before i approve, did you try regenerating the webmock gem rbi?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@markhallen Thank you for the feedback and suggestion.

I did try that, but that did not seem to work

Steps to reproduce:

   rm sorbet/rbi/shims/http_base64.rbi
  bundle exec tapioca gems webmock
rbenv exec bundle exec tapioca gems webmock
Requiring all gems to prepare for compiling...  Done

Removing RBI files of gems that have been removed:

  Nothing to do.


  Compiled webmock
       force  sorbet/rbi/gems/webmock@3.25.1.rbi

Checking generated RBI files...  Done
  No errors found

All operations performed in working directory.

See:

79ef241

https://github.com/dependabot/dependabot-core/actions/runs/21155220687/job/60838734319?pr=13800

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@markhallen Do you have any other feedback about this or any other command I should try?

@yeikel yeikel force-pushed the techdebt/unify-single-http branch 5 times, most recently from 7b41f78 to d3de0ea Compare January 22, 2026 01:44
@yeikel yeikel force-pushed the techdebt/unify-single-http branch 2 times, most recently from c00adb9 to 3a2fbcb Compare January 29, 2026 15:03
Comment on lines 4 to 5
require "sorbet-runtime"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to add a require "excon" here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why do you think that is? I may be missing something but that class takes Dependabot::Service as a constructor argument, and within service there is another injection, but I see no direct references to excon here. Is this class not tested in any other way?

@JamieMagee
Copy link
Copy Markdown
Member

Thanks for doing this! Looks great.

We may want to do a follow-up to investigate using SharedHelpers.excon_defaults, but that shouldn't delay this.

@yeikel yeikel force-pushed the techdebt/unify-single-http branch from 87727f9 to 4990775 Compare January 29, 2026 21:30
@yeikel yeikel force-pushed the techdebt/unify-single-http branch from 4990775 to 1dd17b7 Compare January 29, 2026 21:40
@yeikel
Copy link
Copy Markdown
Contributor Author

yeikel commented Jan 29, 2026

Thanks for doing this! Looks great.

We may want to do a follow-up to investigate using SharedHelpers.excon_defaults, but that shouldn't delay this.

Thanks for the feedback. I wasn’t aware of that helper.

If I’m understanding correctly, you’re referring to this change: 1dd17b7

The repo is currently rate-limited, but the tests are currently running on my fork: yeikel#235

Let me know your preference. We can include it here or I can revert it and handle it separately

@yeikel yeikel closed this Jan 29, 2026
@yeikel yeikel reopened this Jan 29, 2026
@JamieMagee
Copy link
Copy Markdown
Member

This looks fine to me. Thanks again.

@JamieMagee JamieMagee merged commit 6e87159 into dependabot:main Jan 30, 2026
178 of 187 checks passed
@yeikel yeikel deleted the techdebt/unify-single-http branch January 30, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: ruby:bundler RubyGems via bundler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify around a single HTTP library

3 participants