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

stop using UnixMill() to make legacy versions compatible #104

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

convto
Copy link
Contributor

@convto convto commented Oct 26, 2022

The following PR makes use of UnixMill() to get milli-precision timestamps.
#99

UnixMill() was added in go1.17, so it seems that the latest gofrs/uuid is not available from older versions of go.

According to Go's release policy, the last two versions of go are supported.
Therefore, I don't think it is a critical issue that it is not available for go versions older than go1.16.
However, since we expect some users to use older versions of go, we thought it would be better to stop using UnixMill().

@dylan-bourque
Copy link
Member

Everyone should definitely be updating to newer Go versions, but "We want to use time.UnixMilli() internally" doesn't feel like a good reason to force that decision on everyone.

@convto
Copy link
Contributor Author

convto commented Oct 31, 2022

@cameracker
In relation to the PR (#99) I sent out previous, I have made some changes for compatibility from the old Go.

If you have time, I would appreciate your review.
If there are no problems with the PR, please release it.

@cameracker cameracker closed this Oct 31, 2022
@cameracker cameracker reopened this Oct 31, 2022
@cameracker cameracker merged commit e1079f3 into gofrs:master Oct 31, 2022
@cameracker
Copy link
Collaborator

Thanks a ton for identifying this issue and submitting a fix!

@convto convto deleted the make-legacy-versions-compatible branch November 1, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants