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

all timestamps should be pointers #150

Merged
merged 1 commit into from
May 27, 2020
Merged

Conversation

0xch4z
Copy link
Contributor

@0xch4z 0xch4z commented May 27, 2020

This change removes all dereferences of timestamps on resources that don't use pointers.

Sometimes, mysteriously, the API can omit timestamps. With these implementations of UnmarshalJSON, a panic will occur because of a 'nil dereference' (like with terraform-providers/terraform-provider-linode#152).

This change changes all time.Time fields on resources to *time.Time.

@0xch4z 0xch4z requested a review from phillc May 27, 2020 14:33
@phillc phillc requested a review from jnschaeffer May 27, 2020 14:45
@phillc phillc added the bugfix for any bug fixes in the changelog. label May 27, 2020
Copy link
Contributor

@phillc phillc left a comment

Choose a reason for hiding this comment

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

If the api isn't returning a value, then we are assigning the default value of the time type to Created/Updated.
Perhaps instead, the Created/Updated on the struct need to become pointers.
Alternatively/in conjunction we should investigate the scenarios where this happens.

@0xch4z 0xch4z force-pushed the safer-date-parsing branch from 2b1ba64 to e83cc28 Compare May 27, 2020 15:25
@0xch4z 0xch4z changed the title don't dereference timestamps all timestamps should be pointers May 27, 2020
@0xch4z 0xch4z merged commit fc2aea6 into linode:master May 27, 2020
LBGarber pushed a commit to LBGarber/linodego that referenced this pull request Mar 15, 2021
lgarber-akamai pushed a commit to lgarber-akamai/linodego that referenced this pull request Feb 13, 2023
lgarber-akamai pushed a commit to lgarber-akamai/linodego that referenced this pull request Feb 13, 2023
lgarber-akamai pushed a commit to lgarber-akamai/linodego that referenced this pull request Feb 13, 2023
lgarber-akamai pushed a commit to lgarber-akamai/linodego that referenced this pull request Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix for any bug fixes in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants