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 Client.Clone function #774

Merged
merged 3 commits into from Mar 3, 2024
Merged

Conversation

fabiante
Copy link
Contributor

@fabiante fabiante commented Feb 10, 2024

This adds a crude cloning function. It can be used to have a main client with default values and then create copies from that, which must use custom values.

The added function is rather crude and may introduce the potential that, when adding a new Client field, we forget to add it to Clone.

Thus, we should discuss if this is the right way to go. Using code generation or relflection might be a more maintainable solution.

Merging this PR would close #773

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

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

@fabiante Thanks for creating a draft. I have read the issue thread #773.

Can you please update the PR as follows?

Add a new method

// Clone returns a clone of the original client.
//  <add further doc info here>
//
// Since v2.12.0
func (c *Client) Clone() *Client {
	// dereference the pointer and copy the value
	cc := *c
	return &cc
}

Change these field definitions to a pointer type.

udBeforeRequestLock *sync.RWMutex
.
.
.
afterResponseLock   *sync.RWMutex

Finally, initialize the fields while creating a new client

resty/client.go

Line 1343 in 97187c4

func createClient(hc *http.Client) *Client {

@jeevatkm jeevatkm added this to the v2.12.0 milestone Feb 19, 2024
@fabiante fabiante marked this pull request as ready for review February 20, 2024 06:37
client.go Show resolved Hide resolved
client.go Show resolved Hide resolved
Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

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

Thanks, @fabiante, for handling PR comments.

Copy link

codecov bot commented Mar 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.48%. Comparing base (0ac42a2) to head (2c5a4a7).

Additional details and impacted files
@@            Coverage Diff             @@
##               v2     #774      +/-   ##
==========================================
+ Coverage   96.46%   96.48%   +0.01%     
==========================================
  Files          12       12              
  Lines        1643     1650       +7     
==========================================
+ Hits         1585     1592       +7     
  Misses         37       37              
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jeevatkm jeevatkm merged commit 37157fa into go-resty:v2 Mar 3, 2024
3 checks passed
@fabiante fabiante deleted the feat/client-clone branch March 4, 2024 12:11
renovate bot added a commit to Michsior14/transmission-gluetun-port-update that referenced this pull request Mar 18, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [github.com/go-resty/resty/v2](https://togithub.com/go-resty/resty) |
`v2.11.0` -> `v2.12.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-resty%2fresty%2fv2/v2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgo-resty%2fresty%2fv2/v2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgo-resty%2fresty%2fv2/v2.11.0/v2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-resty%2fresty%2fv2/v2.11.0/v2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>go-resty/resty (github.com/go-resty/resty/v2)</summary>

###
[`v2.12.0`](https://togithub.com/go-resty/resty/releases/tag/v2.12.0)

[Compare
Source](https://togithub.com/go-resty/resty/compare/v2.11.0...v2.12.0)

### Release Notes

#### Enhancements

- Add Client.Clone function by
[@&#8203;fabiante](https://togithub.com/fabiante) in
[go-resty/resty#774

#### Bug Fixes

- Fixed JSON request logging data race. by
[@&#8203;buglloc](https://togithub.com/buglloc) in
[go-resty/resty#775
- fix: trailing NULL bytes in buffer while detecting a content type by
[@&#8203;jeevatkm](https://togithub.com/jeevatkm) in
[go-resty/resty#779
- fix: encode path params with BaseURL and the first param at index zero
([#&#8203;781](https://togithub.com/go-resty/resty/issues/781)) by
[@&#8203;sakateka](https://togithub.com/sakateka) in
[go-resty/resty#782

#### Documentation

- fix: doc typo by
[@&#8203;victoraugustolls](https://togithub.com/victoraugustolls) in
[go-resty/resty#769
- docs: replace `SetHostURL` to `SetBaseURL` by
[@&#8203;purofle](https://togithub.com/purofle) in
[go-resty/resty#772
- for v2.12.0 release by
[@&#8203;jeevatkm](https://togithub.com/jeevatkm) in
[go-resty/resty#783

#### New Contributors

- [@&#8203;victoraugustolls](https://togithub.com/victoraugustolls) made
their first contribution in
[go-resty/resty#769
- [@&#8203;purofle](https://togithub.com/purofle) made their first
contribution in
[go-resty/resty#772
- [@&#8203;buglloc](https://togithub.com/buglloc) made their first
contribution in
[go-resty/resty#775
- [@&#8203;fabiante](https://togithub.com/fabiante) made their first
contribution in
[go-resty/resty#774
- [@&#8203;sakateka](https://togithub.com/sakateka) made their first
contribution in
[go-resty/resty#782

**Full Changelog**:
go-resty/resty@v2.11.0...v2.12.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Michsior14/transmission-gluetun-port-update).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNDUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjI0NS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Clone or copy of existing client
2 participants