feat: add/use RFC3339 fields in requests #233
Conversation
| @Value | ||
| @Builder | ||
| public class PostFeedbackRequestBody { | ||
| FeedbackEvent event; | ||
| Long timestamp; | ||
| Instant occurredAt; |
There was a problem hiding this comment.
This is a breaking change once PostFeedbackRequestBody is public, but I'm considering it as not because the user isn't supposed to use this class directly. Should change the visiblity of these internal classes to avoid issues?
There was a problem hiding this comment.
I'll change it in another PR to be released together with the breaking changes.
figueredo
force-pushed
the
rfc3339-fields
branch
from
8401bc0 to
33c576a
Compare
| @Deprecated Long timestamp; | ||
| Instant collectedAt; |
There was a problem hiding this comment.
I decided to mark the old field as deprecated and add the new here as this class is used by the user. Given that we are bumping the major due to the removal of GetSignup, should we take the opportunity and remove the old field too?
There was a problem hiding this comment.
I'll remove it later together with the other breaking changes.
figueredo
force-pushed
the
rfc3339-fields
branch
3 times, most recently
from
7b4e6d8 to
43ff19b
Compare
Timestamp fields that store time as milliseconds since epoch are being replaced by fields that store it as a RFC 3339 string. This commit: - Removes POST /feedbacks `timestamp` field, replacing it with `occurred_at`. Removal is possible because this field is not in the public API. - Add `collected_at` to POST /signups' `additional_locations`. This field should be used instead of the existing `timestamp`.
figueredo
force-pushed
the
rfc3339-fields
branch
from
43ff19b to
3285df2
Compare
Proposed changes
Timestamp fields that store time as milliseconds since epoch are being replaced by fields that store it as a RFC 3339 string.
This commit:
timestampfield, replacing it withoccurred_at. Removal is possible because this field is not in the public API.collected_atto POST /signups'additional_locations. This field should be used instead of the existingtimestamp.Checklist