-
Notifications
You must be signed in to change notification settings - Fork 572
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 current_period_*
filters when listing Invoices
#1547
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment, but otherwise LGTM.
ptal @remi-stripe
/// A filter on the list based on the object current_period_end field. | ||
/// </summary> | ||
[JsonProperty("current_period_end")] | ||
public DateRangeOptions CurrentPeriodEndRange{ get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite minor, but this could use another space between Range
and the opening curly brace.
/// A filter on the list based on the object current_period_end field. | ||
/// </summary> | ||
[JsonProperty("current_period_start")] | ||
public DateRangeOptions CurrentPeriodStartRange{ get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also missing space here similar to the above.
public DateRangeOptions CurrentPeriodEndRange{ get; set; } | ||
|
||
/// <summary> | ||
/// A filter on the list based on the object current_period_end field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, and actually one more: this comment and the one below it read current_period_end
when they should read current_period_start
.
97af68f
to
9e55bb2
Compare
@brandur-stripe damn really sorry to have had 3 issues in such a small PR :(. Fixed, PTAL |
9e55bb2
to
3d18eae
Compare
3d18eae
to
6a824f5
Compare
Hah, no worries! And actually, just looking at AppVeyor, one more:
You're probably just missing a |
Yeah, for some reason my amend never made it to the 3 push I tried. But fixed it now, sorry for the chunder. |
LGTM. |
r? @brandur-stripe
cc @stripe/api-libraries