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

chore: use Ecto.Type.type to determine db type #81

Merged
merged 2 commits into from
Aug 16, 2024
Merged

Conversation

BruceBC
Copy link

@BruceBC BruceBC commented Aug 14, 2024

Context

ecto release v1.12.1 introduced a change to the way mappings were return for Ecto.Enum.mappings/2.

We were relying on this mapping to convert Ecto.Enum to :string when creating the database columns.

See elixir-ecto/ecto@21c6068#diff-025069044568b480ef6f86f025cb46a7064695e6df1e7008ede689e09a9c7836L317-R318

Decision

To make this more future proof, use Ecto.Type.type/1 to handle schema type to database type conversions.


chore: fix single-quoted warning

Context

warning: single-quoted strings represent charlists. Use ~c"" if you indeed want a charlist or use "" instead
     │
 256 │   defp insert_all_value(_), do: '?'
     │                                 ~
     │
     └─ lib/snowflex/ecto/connection.ex:256:33

     warning: single-quoted strings represent charlists. Use ~c"" if you indeed want a charlist or use "" instead
     │
 601 │     '?'
     │     ~
     │
     └─ lib/snowflex/ecto/connection.ex:601:5

## Context

```
warning: single-quoted strings represent charlists. Use ~c"" if you indeed want a charlist or use "" instead
     │
 256 │   defp insert_all_value(_), do: '?'
     │                                 ~
     │
     └─ lib/snowflex/ecto/connection.ex:256:33

     warning: single-quoted strings represent charlists. Use ~c"" if you indeed want a charlist or use "" instead
     │
 601 │     '?'
     │     ~
     │
     └─ lib/snowflex/ecto/connection.ex:601:5
```
## Context

ecto release v1.12.1 introduced a change to the way mappings were return for `Ecto.Enum.mappings/2`.

We were relying on this mapping to convert Ecto.Enum to `:string` when creating the database columns.

See elixir-ecto/ecto@21c6068#diff-025069044568b480ef6f86f025cb46a7064695e6df1e7008ede689e09a9c7836L317-R318

## Decision

To make this more future proof, use `Ecto.Type.type/1` to handle schema type to database type conversions.
@BruceBC BruceBC requested review from amaczu and caleb-acosta August 14, 2024 19:35
@PM-Pepsico
Copy link
Contributor

Very nice! I like it.
I wasn't sure if we would want a version bump in the mix.exs but it looks like Ecto.Type.type has been in since at least 3.0.0.

@BruceBC BruceBC merged commit 77a50be into v1.0.0 Aug 16, 2024
BruceBC added a commit that referenced this pull request Aug 19, 2024
…ixes"

This reverts commit 77a50be, reversing
changes made to b6eb28c.
BruceBC added a commit that referenced this pull request Aug 19, 2024
…ges-ecto-type

Revert "Merge pull request #81"
PM-Pepsico pushed a commit that referenced this pull request Aug 27, 2024
…ixes"

This reverts commit 77a50be, reversing
changes made to b6eb28c.
PM-Pepsico pushed a commit that referenced this pull request Aug 27, 2024
…ixes"

This reverts commit 77a50be, reversing
changes made to b6eb28c.
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.

2 participants