[PR #202] [MERGED] Remove Rspotify default parameters and add parameter macros #311

New issue

Closed

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

kerem commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/202
Author: @marioortizmanero
Created: 4/19/2021
Status: ✅ Merged
Merged: 4/26/2021
Merged by: @ramsayleung

Base: master ← Head: default

📝 Commits (10+)

5e21000 lots of unwrap_or removed
3d183c6 initial macro
eee8ceb macros
8a3fccf json macro
6eacf77 small simplification
251e80b add support for simple parameters
60e8628 added docs and tests to macros
9311555 fix tests
fd4866a fixed some examples
b603d54 cleanup test

📊 Changes

11 files changed (+634 additions, -536 deletions)

View changed files

📝 CHANGELOG.md (+6 -1)
📝 examples/current_user_recently_played.rs (+1 -1)
📝 examples/pagination_manual.rs (+1 -1)
📝 examples/ureq/device.rs (+1 -3)
📝 examples/ureq/me.rs (+1 -3)
📝 examples/ureq/search.rs (+17 -12)
📝 src/client.rs (+397 -458)
📝 src/macros.rs (+156 -18)
📝 src/model/enums/misc.rs (+1 -4)
📝 tests/test_with_credential.rs (+1 -1)
📝 tests/test_with_oauth.rs (+52 -34)

📄 Description

Description

Leaves default parameter handling to Spotify, removing .unwrap_or
Implements a macro for both query and json parameters so that endpoints are much nicer to read.
Closes #134, we're consistently using Option<T> instead of Into<Opton<T>> now. If you don't like this please let me know, but I didn't get any more feedback so I went for the simpler option.

Motivation and Context

Default values should be assigned by the Spotify API instead by us, clearly. We shouldn't touch things we don't have to.
As #127 said, endpoints should be as concise as possible because they're the most important part of the library after all, and it's important that they are easy to maintain.

Dependencies

None

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Please also list any relevant details for your test configuration

There are a couple new tests for the macros

_{🔄 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/202 **Author:** [@marioortizmanero](https://github.com/marioortizmanero) **Created:** 4/19/2021 **Status:** ✅ Merged **Merged:** 4/26/2021 **Merged by:** [@ramsayleung](https://github.com/ramsayleung) **Base:** `master` ← **Head:** `default` --- ### 📝 Commits (10+) - [`5e21000`](https://github.com/ramsayleung/rspotify/commit/5e210004f8fda311751d5f845e47efc538d13981) lots of unwrap_or removed - [`3d183c6`](https://github.com/ramsayleung/rspotify/commit/3d183c6a5d69a4e0a2544064d88da51876332c8c) initial macro - [`eee8ceb`](https://github.com/ramsayleung/rspotify/commit/eee8ceb1f3e22fdf430330596a6add54575b0abf) macros - [`8a3fccf`](https://github.com/ramsayleung/rspotify/commit/8a3fccf0c7371ee280a234fda7bf9a1e8850d385) json macro - [`6eacf77`](https://github.com/ramsayleung/rspotify/commit/6eacf77b371d5364439bb0ceb4acf157b0738b2c) small simplification - [`251e80b`](https://github.com/ramsayleung/rspotify/commit/251e80bf0471a8b33b061e9375792bcb04c86a77) add support for simple parameters - [`60e8628`](https://github.com/ramsayleung/rspotify/commit/60e8628d2d1c1efefefd08c5a42536aa333f707b) added docs and tests to macros - [`9311555`](https://github.com/ramsayleung/rspotify/commit/93115553313b0046c6a34cd3d4049feaa287e20c) fix tests - [`fd4866a`](https://github.com/ramsayleung/rspotify/commit/fd4866aa92204ebc34868eaf8a27778232096f83) fixed some examples - [`b603d54`](https://github.com/ramsayleung/rspotify/commit/b603d5437c91c9eaf79c1bb0ad7f629ce4906379) cleanup test ### 📊 Changes **11 files changed** (+634 additions, -536 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+6 -1) 📝 `examples/current_user_recently_played.rs` (+1 -1) 📝 `examples/pagination_manual.rs` (+1 -1) 📝 `examples/ureq/device.rs` (+1 -3) 📝 `examples/ureq/me.rs` (+1 -3) 📝 `examples/ureq/search.rs` (+17 -12) 📝 `src/client.rs` (+397 -458) 📝 `src/macros.rs` (+156 -18) 📝 `src/model/enums/misc.rs` (+1 -4) 📝 `tests/test_with_credential.rs` (+1 -1) 📝 `tests/test_with_oauth.rs` (+52 -34) </details> ### 📄 Description ## Description * Leaves default parameter handling to Spotify, removing `.unwrap_or` * Implements a macro for both query and json parameters so that endpoints are much nicer to read. * Closes #134, we're consistently using `Option<T>` instead of `Into<Opton<T>>` now. If you don't like this please let me know, but I didn't get any more feedback so I went for the simpler option. ## Motivation and Context * Default values should be assigned by the Spotify API instead by us, clearly. We shouldn't touch things we don't have to. * As #127 said, endpoints should be as concise as possible because they're the most important part of the library after all, and it's important that they are easy to maintain. ## Dependencies None ## Type of change Please delete options that are not relevant. - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration - [x] There are a couple new tests for the macros --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>