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

[DevExperience]: Inconsistency in time related field types #2645

Closed
zombieleet opened this issue Jan 25, 2023 · 2 comments
Closed

[DevExperience]: Inconsistency in time related field types #2645

zombieleet opened this issue Jan 25, 2023 · 2 comments
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Comments

@zombieleet
Copy link
Contributor

I am currently working with go-github sdk, and I noticed some inconsistencies in the types of time fields. Some fields are *time.Time while others are *Timestamp.

For example, this difference can be spotted in the PullRequest and Projects struct. It is also littered around the codebase. In my opinion, let's stick to one type for time fields which is *Timestamp.

if this is seen as a valid issue, I will open a PR to fix it.

Thanks.

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 25, 2023

Thank you, @zombieleet .
It seems like there was a discussion about this before, but I'm not finding it.

Consistency would be nice, and I can't think of any downsides other than a boat-load of breaking API changes.
But that's why we version this, and people can hold back until they feel like upgrading.

So I have no objections to a PR.

@gmlewis gmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label Jan 25, 2023
@gmlewis
Copy link
Collaborator

gmlewis commented Jan 26, 2023

Fixed by: #2646
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).
Projects
None yet
Development

No branches or pull requests

2 participants