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

Add ConsulBuilder allowing a consul client to be built using a custom HTTP connector, fixes #44 #45

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

LeonHartley
Copy link
Contributor

@LeonHartley LeonHartley commented Apr 22, 2024

What problem are we solving?

Issue #44

How are we solving the problem?

Create a new consul client builder allowing a custom HTTP client to be supplied, whilst keeping current API as-is so as to not introduce any breaking changes.

Checks

Please check these off before promoting the pull request to non-draft status.

  • All CI checks are green.
  • I have reviewed the proposed changes myself.

Copy link

github-actions bot commented Apr 22, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@LeonHartley
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@kushudai
Copy link
Contributor

Hi @LeonHartley
Thank you for the issue and the PR!

This mostly looks good to me. I have 2 relatively small (I hope) asks:

  • Could we take the hyperBuilder currently in the config and also move it to ConsulBuilder?
  • Could you also run cargo fmt to fix the lint?

@kushudai kushudai self-requested a review April 27, 2024 02:03
@kushudai kushudai merged commit 6645745 into Roblox:main Jul 20, 2024
5 of 6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants