Skip to content

Update HTTP client config in factory to allow for controlling IP resolution #7867

Merged
techanvil merged 5 commits intodevelopfrom
enhance/7864-ip-resolve
Nov 22, 2023
Merged

Update HTTP client config in factory to allow for controlling IP resolution #7867
techanvil merged 5 commits intodevelopfrom
enhance/7864-ip-resolve

Conversation

@aaemnnosttv
Copy link
Copy Markdown
Collaborator

@aaemnnosttv aaemnnosttv commented Nov 13, 2023 •
edited by techanvil
Loading

Summary

Addresses issue:

Relevant technical choices

PR Author Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 5.2 and PHP 5.6.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have added a QA Brief on the issue linked above.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

  • Run the code.
  • Ensure the acceptance criteria are satisfied.
  • Reassess the implementation with the IB.
  • Ensure no unrelated changes are included.
  • Ensure CI checks pass.
  • Check Storybook where applicable.
  • Ensure there is a QA Brief.

Merge Reviewer Checklist

  • Ensure the PR has the correct target branch.
  • Double-check that the PR is okay to be merged.
  • Ensure the corresponding issue has a ZenHub release assigned.
  • Add a changelog message to the issue.

@aaemnnosttv aaemnnosttv mentioned this pull request Nov 13, 2023
Closed
@eugene-manuilov eugene-manuilov marked this pull request as ready for review November 21, 2023 16:42
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 21, 2023 •
edited
Loading

Build files for 440d1f5 have been deleted.

techanvil
techanvil reviewed Nov 22, 2023
View reviewed changes
tests/phpunit/integration/Core/Authentication/Clients/Client_FactoryTest.php Outdated
/**
* @dataProvider data_config_ip_resolve_values
*/
public function test_client_ip_resolve_config_options( $ip_resolve, $want ) {
Copy link
Copy Markdown
Collaborator

@techanvil techanvil Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's a minor point, but let's use $expected_value rather than $want, it would be more consistent with other tests.

I've made the change myself to save the round trip.

Copy link
Copy Markdown
Collaborator Author

@aaemnnosttv aaemnnosttv Nov 30, 2023

Choose a reason for hiding this comment

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

Golang habits 😄

techanvil
techanvil approved these changes Nov 22, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@techanvil techanvil left a comment

Choose a reason for hiding this comment

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

LGTM!

@techanvil techanvil merged commit c9bd883 into develop Nov 22, 2023
@techanvil techanvil deleted the enhance/7864-ip-resolve branch November 22, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@techanvil techanvil techanvil approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aaemnnosttv @techanvil