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 unused Http proxy class #2763

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Remove unused Http proxy class #2763

merged 1 commit into from
Jan 14, 2025

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Jan 13, 2025

Changes proposed in this Pull Request:

The Http proxy class was initially added in PR #69 with the comment:

A Proxy for making HTTP requests to the connect server in the future.

In the end we decided to go with a Guzzle client to send requests to the connect server. So in the interest of reducing code this PR removes this class. If we change our mind this PR can be reverted to bring it back.

Detailed test instructions:

  1. Run some basic smoke tests to confirm API requests continue to work.
  2. Run unit tests to confirm they all pass.

Changelog entry

  • Tweak - Remove unused Http proxy class.

@mikkamp mikkamp self-assigned this Jan 13, 2025
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jan 13, 2025
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.1%. Comparing base (c8c1efa) to head (105ce3b).
Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             develop   #2763     +/-   ##
===========================================
+ Coverage       67.0%   67.1%   +0.1%     
+ Complexity      4692    4687      -5     
===========================================
  Files            480     479      -1     
  Lines          19601   19586     -15     
===========================================
  Hits           13141   13141             
+ Misses          6460    6445     -15     
Flag Coverage Δ
php-unit-tests 67.1% <ø> (+0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rnal/DependencyManagement/ProxyServiceProvider.php 0.0% <ø> (ø)

@mikkamp mikkamp requested a review from a team January 13, 2025 15:11
Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

Thanks @mikkamp LGTM ✅

  • Tested there is no more Proxies\Http usage. Now I can only see Guzzle\Http and Psr\Http
  • Tested a full onboarding
  • Tested updating Shipping settings
  • Checked for fatals or warnings in logs

@mikkamp mikkamp merged commit 94b372e into develop Jan 14, 2025
15 checks passed
@mikkamp mikkamp deleted the tweak/remove-http-proxy branch January 14, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants