helper/validation: UUID -> IsUUID #297
Conversation
|
I personally would say |
|
No problem - I will change it @katbyte |
helper/validation/uuid.go
Outdated
| // UUIDRegExp is a Regular Expression that can be used to validate UUIDs | ||
| var UUIDRegExp = regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$") | ||
| // uuidRegExp is a Regular Expression that can be used to validate UUIDs | ||
| var uuidRegExp = regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$") |
There was a problem hiding this comment.
I think this was intentionally exported in case a developer ever wanted a regex pattern for UUID, even though we don't use it internally
There was a problem hiding this comment.
Well, in my mind it's an implementation detail in the context of the validation package which is supposed to provide validation functions and anything exported is a liability for potential stability, so that's why I'm thinking that this gives us some headroom to move this around more easily without having to break anything/anyone.
There was a problem hiding this comment.
Ah, I should have made it more clear, that this regex actually ISN'T used in the implementation (the related function just uses go's native parser). I believe Katie just threw in the Regex if it provided value to developers looking to use the regex. I gave her the unofficial "yeah why not throw it in" so that is my bad. We might want to just delete it for now and publish it in a future release should there be demand for it (also maybe it doesn't belong in validation 🤔 )
There was a problem hiding this comment.
also maybe it doesn't belong in validation
That's my thinking. It would be very difficult to maintain any stability guarantees, because it's essentially a constant, so every time you change it you break it.
There was a problem hiding this comment.
Okay, let's just delete it for now and bring it up again if it's needed later. cc @katbyte
rename IsValidUUID to IsUUID
appilon
changed the title
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Just a (nitpicky) patch to align the function name with others and clarify the purpose of the function a bit.
I would be also 👌 with
IsUUIDorValidUUID, but the function name should IMO communicate that it's positive validation.