[PR #330] [MERGED] Avoid accepting references to `Copy` types #400

New issue

Closed

opened 2026-02-27 20:24:35 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:24:35 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/330
Author: @SabrinaJewson
Created: 6/15/2022
Status: ✅ Merged
Merged: 6/15/2022
Merged by: @marioortizmanero

Base: master ← Head: no-copy-ref

📝 Commits (2)

d2e798b Avoid accepting references to Copy types
4a948df Update changelog

📊 Changes

4 files changed (+19 additions, -16 deletions)

View changed files

📝 CHANGELOG.md (+3 -0)
📝 src/clients/base.rs (+3 -3)
📝 src/clients/oauth.rs (+10 -10)
📝 tests/test_with_oauth.rs (+3 -3)

📄 Description

Description

search accepts Option<IncludeExternal> instead of Option<&IncludeExternal>
featured_playlists accepts Option<chrono::DateTime<chrono::Utc>> instead of Option<&chrono::DateTime<chrono::Utc>>
current_user_top_artists[_manual] and current_user_top_tracks[_manual] accept Option<TimeRange> instead of Option<&TimeRange>

I didn’t do the same for Market parameters because they’re not Copy (although they could be) — I’ll leave it to a separate PR.

Motivation and Context

Accepting a reference to a Copy type is usually counterproductive, since you can just take it by value. For most types this makes the size of the parameters smaller, for the Option<chrono::DateTime<chrono::Utc>> it increases it from 8 bytes to 16 bytes but I think that’s acceptable.

Dependencies

None.

Type of change

Please delete options that are not relevant.

Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

Test A: cargo test --features env-file

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/330 **Author:** [@SabrinaJewson](https://github.com/SabrinaJewson) **Created:** 6/15/2022 **Status:** ✅ Merged **Merged:** 6/15/2022 **Merged by:** [@marioortizmanero](https://github.com/marioortizmanero) **Base:** `master` ← **Head:** `no-copy-ref` --- ### 📝 Commits (2) - [`d2e798b`](https://github.com/ramsayleung/rspotify/commit/d2e798bd9748a8379e40b41c2f23ce5e170456ae) Avoid accepting references to `Copy` types - [`4a948df`](https://github.com/ramsayleung/rspotify/commit/4a948df3353a1cbfbd646a738ddf9cde0d83c411) Update changelog ### 📊 Changes **4 files changed** (+19 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+3 -0) 📝 `src/clients/base.rs` (+3 -3) 📝 `src/clients/oauth.rs` (+10 -10) 📝 `tests/test_with_oauth.rs` (+3 -3) </details> ### 📄 Description ## Description - `search` accepts `Option<IncludeExternal>` instead of `Option<&IncludeExternal>` - `featured_playlists` accepts `Option<chrono::DateTime<chrono::Utc>>` instead of `Option<&chrono::DateTime<chrono::Utc>>` - `current_user_top_artists[_manual]` and `current_user_top_tracks[_manual]` accept `Option<TimeRange>` instead of `Option<&TimeRange>` I didn’t do the same for `Market` parameters because they’re not `Copy` (although they could be) — I’ll leave it to a separate PR. ## Motivation and Context Accepting a reference to a `Copy` type is usually counterproductive, since you can just take it by value. For most types this makes the size of the parameters smaller, for the `Option<chrono::DateTime<chrono::Utc>>` it increases it from 8 bytes to 16 bytes but I think that’s acceptable. ## Dependencies None. ## Type of change Please delete options that are not relevant. - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) ## How has this been tested? - [x] Test A: `cargo test --features env-file` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>