Skip to content

refactor(go): refactor WithTLS#2957

Merged
mmodzelewski merged 3 commits intoapache:masterfrom
chengxilo:refactor-with-tls
Mar 17, 2026
Merged

refactor(go): refactor WithTLS#2957
mmodzelewski merged 3 commits intoapache:masterfrom
chengxilo:refactor-with-tls

Conversation

@chengxilo
Copy link
Contributor

@chengxilo chengxilo commented Mar 17, 2026

Which issue does this PR close?

N/A

Rationale

Currently, the TLS options requiring a user to call WithTLS(true) in addition to WithTLSDomain or WithTLSCAFile. This is redundant and error-prone. If a user provides a domain or a CA file, the intent to use TLS is already clear.

We can use nested option pattern for it, therefore we can create client like this:

cli, err := client.NewIggyClient(
	client.WithTcp(
		tcp.WithServerAddress(connectAddr),
		tcp.WithTLS(
			tcp.WithTLSCAFile(caFile),
			tcp.WithTLSDomain("localhost"),
		),
	),
)

What changed?

  1. Refactor WithTLS method.
  2. Extract TLS configuration struct, group all TLS-related parameters together

Local Execution

  • Passed
  • Pre-commit hooks ran

AI Usage

  1. Claude Sonnet 4.6
  2. code generation
  3. I reviewed the code and tested
  4. Sure

@chengxilo chengxilo marked this pull request as draft March 17, 2026 00:56
@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 37.93103% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.47%. Comparing base (79103ee) to head (f0e754b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
foreign/go/client/tcp/tcp_core.go 37.93% 18 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2957      +/-   ##
============================================
+ Coverage     70.35%   70.47%   +0.11%     
+ Complexity      925      776     -149     
============================================
  Files          1050     1041       -9     
  Lines         86936    86181     -755     
  Branches      64491    62443    -2048     
============================================
- Hits          61167    60734     -433     
+ Misses        23260    22931     -329     
- Partials       2509     2516       +7     
Flag Coverage Δ
go 36.38% <37.93%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
foreign/go/client/tcp/tcp_core.go 47.80% <37.93%> (+0.05%) ⬆️

... and 259 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chengxilo chengxilo marked this pull request as ready for review March 17, 2026 01:19
@mmodzelewski mmodzelewski merged commit c70ff95 into apache:master Mar 17, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants