Skip to content
This repository was archived by the owner on Dec 7, 2020. It is now read-only.

Conversation

@abstractj
Copy link

Fixes #611

JoelSpeed
JoelSpeed previously approved these changes May 13, 2020
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

This makes sense. Must seems a bit weird, are there any ways we could get around this? Should we follow up on that at some point? Could create an issue for now, it's not blocking

@abstractj
Copy link
Author

This makes sense. Must seems a bit weird, are there any ways we could get around this? Should we follow up on that at some point? Could create an issue for now, it's not blocking

@JoelSpeed the other alternative is to do the traditional error handling. Something like:

uuid, err := uuid.NewV4()
if err !=nil {
//handle the error here
}
//do something with uuid.String()

A bit more verbose, but if you have a strong feeling about using Must, I can do the refactor. Wdyt?

@JoelSpeed
Copy link
Contributor

It would be more correct to take the error and return a 500 I would say

@abstractj
Copy link
Author

@JoelSpeed ready for another round.

Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

LGTM

@abstractj abstractj merged commit 3905412 into louketo:master May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace satori/go.uuid by gofrs/uuid

2 participants