search

gofrs / uuid

A UUID package for Go
MIT License
1.57k stars 110 forks source link

Why does Timestamp.Time return an error? #87

Closed esco closed 3 years ago

esco commented 3 years ago

When parsing a time.Time value from a v1 uuid using UUID.TimestampFromV1, the Timestamp.Time function is called. This function only ever returns a nil error. Why does this function have an error for the second return argument when one can't ever be produced?

https://github.com/gofrs/uuid/blob/094e8c20495e85b2066fb9e69159b0f3fc066bca/uuid.go#L72-L85

theckman commented 3 years ago

Looks like it was kept in a refactor from an external contribution, and missed in review.

zerkms commented 3 years ago

@theckman I asked you about it in slack a year (?) ago (when the project was initially forked) and your response was to have it here just in case if it ever needed to not break the API.

theckman commented 3 years ago

I thought that was the case, but couldn't find the conversation so I didn't want to commit to that.

esco commented 3 years ago

I see, @theckman would you say it's safe to simply use a blank identifier for the error value and proceed as if an error isn't possible?

theckman commented 3 years ago

@esco I wouldn't be comfortable saying that. Since it wouldn't require an API change, I could see us making a change that could cause that to start returning an error. In that case a minor version upgrade would break your code.

The assumption is that Gophers handle any error values that are returned to them, and so under SemVer having this return more errors would probably be considered a backwards compatible change.

esco commented 3 years ago

Makes sense. Thanks for the insight and quick response!

cameracker commented 3 years ago

Hi @esco , thank you very much for the question. Its looking like this discussion is resolved (for now) so I'm going to close the issue. If you have anything else you'd like to enquire about this topic please feel free to keep posting!

Have a good one