fix: #578 by allowing arbitrary strings for PriceIds #580
Conversation
Cargo.toml
Outdated
| @@ -7,7 +7,7 @@ members = [ | |||
|
|
|||
| [package] | |||
| name = "async-stripe" | |||
| version = "0.37.2" | |||
| version = "0.37.3" | |||
There was a problem hiding this comment.
I don't think you need to manually bump this. This will probably be taken care when a new version is created from master.
There was a problem hiding this comment.
Correct (and thanks for reviewing @augustoccesar!) please revert this line. You should instead prefix your commit message with fix: and semantic versioning will handle the rest.
src/ids.rs
Outdated
| @@ -555,7 +555,7 @@ def_id!( | |||
| def_id!(PersonId, "person_"); | |||
| def_id!(PlanId: String); // N.B. A plan id can be user-provided so can be any arbitrary string | |||
| def_id!(PlatformTaxFeeId, "ptf"); | |||
| def_id!(PriceId, "price_" | "plan_"); // see #470 | |||
| def_id!(PriceId, "price_" | ""); // see #470 | |||
There was a problem hiding this comment.
Since this will basically behave like the PlanId and ProductId, that being: "it can be any arbitrary string", I would say that maybe it should be the same as them?
So something like:
| def_id!(PriceId, "price_" | ""); // see #470 | |
| def_id!(PriceId, String); |
Also not sure if the link to the issue #470 here will make sense anymore. But could be nice to add a comment explaining about why the PriceId can be any String? Or a link to your original issue. Wdyt?
There was a problem hiding this comment.
Yes, please change this to a string and add a comment. Thanks!
src/ids.rs
Outdated
| @@ -555,7 +555,7 @@ def_id!( | |||
| def_id!(PersonId, "person_"); | |||
| def_id!(PlanId: String); // N.B. A plan id can be user-provided so can be any arbitrary string | |||
| def_id!(PlatformTaxFeeId, "ptf"); | |||
| def_id!(PriceId, "price_" | "plan_"); // see #470 | |||
| def_id!(PriceId, "price_" | ""); // see #470 | |||
There was a problem hiding this comment.
Yes, please change this to a string and add a comment. Thanks!
thomasmost
changed the title
|
Running CI, TY for being so responsive. |
|
@thomasmost fyi, the Clippy CI issues should be fixed if you rebase the branch 🙏 |
|
@augustoccesar whoops — will that merge do the trick? |
|
@thomasmost that should do it! |
|
Running CI now will merge when done. TY! |
|
All merged, Thank you for taking the time. |
|
🎉 This PR is included in version 0.38.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
@arlyon Thanks for maintaining this crate! |
Summary
Fixes #578
Checklist
cargo make fmt