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 a long list of missing params and properties #820

Merged
merged 1 commit into from
Mar 27, 2019

Conversation

remi-stripe
Copy link
Contributor

r? @brandur-stripe
cc @mickjermsurawong-stripe who found all of those for us.
cc @stripe/api-libraries

Params `form:"*"`
PersonalIDNumber *string `form:"personal_id_number"`
Params `form:"*"`
IDNumber *string `form:"id_number"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a breaking change and likely requires a major version to play it safe even if it likely did not work.

Copy link
Contributor

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

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

All changes look good to me Remi!

And wow — it's so cool that we caught all these before a user noticed and complained about any of them! WE (finally) HAVE THE TECHNOLOGY.

refund.go Outdated
@@ -38,6 +38,7 @@ type RefundParams struct {
// For more details see https://stripe.com/docs/api#list_refunds.
type RefundListParams struct {
ListParams `form:"*"`
Charge *string `form:"charge"`
Copy link
Contributor

Choose a reason for hiding this comment

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

@remi-stripe remi-stripe force-pushed the remi-add-missing-properties branch from 2e49535 to 83216d4 Compare March 27, 2019 20:30
@mickjermsurawong-stripe
Copy link
Contributor

mickjermsurawong-stripe commented Mar 27, 2019

@brandur-stripe, would like your thoughts on representing empty string as a way to unset params please? These are a pattern of params we currently don't support in Go but it's in OpenAPI.
This corresponds to our backend param with :allow_present_but_empty but it can have different semantic based on endpoint. For example, billing_thresholds
https://stripe.com/docs/api/subscriptions/update#update_subscription-billing_thresholds

Pass an empty string to remove previously-defined thresholds.

On a separate note, I would like to keep this PR opened for at least this week, in case I missed flagging params among things we decided to actually not support and the empty string param.

@remi-stripe
Copy link
Contributor Author

@mickjermsurawong-stripe IMO we should not keep such a PR open for a week. There's no real upside to "waiting until we find more", we would just do a brand new PR at that point.

I also think the empty params should be resolved entirely separately and not in this PR. We should also talk about the best approach internally instead.

@mickjermsurawong-stripe
Copy link
Contributor

ah that's fair. I'm good with merging this then
LGTM

@brandur-stripe
Copy link
Contributor

I also think the empty params should be resolved entirely separately and not in this PR. We should also talk about the best approach internally instead.

+1. I think we're going to need to add a new property (e.g. Empty) and custom encode function to support this properly unfortunately.

@mickjermsurawong-stripe
Copy link
Contributor

The build was stuck but seems working now. I can take the release

@remi-stripe
Copy link
Contributor Author

@brandur-stripe yeah I canceled and restarted. Leaving the release to you, thanks a lot! Reminder that it's a major version to be safe

@brandur-stripe
Copy link
Contributor

Released as 60.0.0.

@mickjermsurawong-stripe
Copy link
Contributor

mickjermsurawong-stripe commented Mar 27, 2019

ah thank you @brandur-stripe! i'm just about reading instructions and preparing the release message!

@brandur-stripe
Copy link
Contributor

ah thank you @brandur-stripe! i'm just about reading instructions and preparing the release message!

Oops, sorry @mickjermsurawong-stripe — I didn't mean to step on your toes. Somehow I read the "you" in as meaning me, hah (which reading through again makes no sense I see).

@remi-stripe remi-stripe deleted the remi-add-missing-properties branch November 25, 2019 16:54
nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024
Bumps [selenium-webdriver](https://github.com/SeleniumHQ/selenium) from 4.4.0 to 4.5.0.
- [Release notes](https://github.com/SeleniumHQ/selenium/releases)
- [Changelog](https://github.com/SeleniumHQ/selenium/blob/trunk/rb/CHANGES)
- [Commits](SeleniumHQ/selenium@selenium-4.4.0...selenium-4.5.0)

---
updated-dependencies:
- dependency-name: selenium-webdriver
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants