Skip to content

Replace UUID.uuid4/0 by Ecto.UUID.generate/0#14

Merged
sax merged 1 commit into
absinthe-graphql:mainfrom
mathieuprog:main
Jul 29, 2022
Merged

Replace UUID.uuid4/0 by Ecto.UUID.generate/0#14
sax merged 1 commit into
absinthe-graphql:mainfrom
mathieuprog:main

Conversation

@mathieuprog

Copy link
Copy Markdown
Contributor

The elixir-uuid project is dead: zyro/elixir-uuid#56

From Elixir 1.12, that lib generates a warning.

Replace it by Ecto.UUID.generate/0? Elixir projects depend already on Ecto.

@sax
sax merged commit af6a257 into absinthe-graphql:main Jul 29, 2022
@sax

sax commented Jul 29, 2022

Copy link
Copy Markdown
Contributor

Sorry for the delay! Lost track of this among other work I've been doing.

@maartenvanvliet

Copy link
Copy Markdown
Contributor

I think it's a bit much to include Ecto just for the sake of generating UUIDs. It could be an optional dependency or just copy over some code.

@sax

sax commented Jul 29, 2022

Copy link
Copy Markdown
Contributor

I was also considering using uniq, but then realized that that depends on ecto as well.

@sax

sax commented Jul 29, 2022

Copy link
Copy Markdown
Contributor

@maartenvanvliet I think having it as an optional (or configurable) dependency adds more complication, as ecto's function is generate/0 whereas other implementations have uuid4/0. I've gone with your suggestion of copying in some minimal code with a notice about where it was derived from.

@mathieuprog

Copy link
Copy Markdown
Contributor Author

What do you think would be the downsides of having Ecto as a dependency? Ofc I understand it's a huge library to depend on just to get a single utility from that lib. But pragmatically most projects depend on Ecto, also one doesn't have to start the Ecto application, and in the end, isn't it just some files copied? There is the dependency version to manage, but this utility function won't change so we can be open to any version of Ecto.

Just curious about your thoughts.

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