Skip to content

Misc improvements from mochaa/main - #184

Open
mochaaP wants to merge 7 commits into
elixir-sqlite:mainfrom
mcha-forks:main
Open

mochaaP wants to merge 7 commits into
elixir-sqlite:mainfrom
mcha-forks:main

Conversation

@mochaaP

@mochaaP mochaaP commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Note: do not squash merge this PR. Commits in this PR are self-contained and atomic, so squash merging breaks the semantics.

Namely, this branch implements check constraints via Ecto.Migration.constraint/3.

@mochaaP

mochaaP commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

closes #178

@mochaaP

mochaaP commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

An unrelated TODO: missing test for column-level CHECK constraints.

@warmwaffles

Copy link
Copy Markdown
Member

The reason I squash merge is that an overwhelming majority of people who contribute to libraries are not very commit hygienic. They don't usually rebase or clean up their work for whatever reason. So squashing is easier for me to manage, and revert whole sale if something was horridly wrong.

I can rebase merge this, so it's not a big deal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I intentionally kept the create and create_if_not_exists separate for the executeddl stuff. Ya it's repeated, but it's clear what the function is for.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was copied from ecto_sql. I could revert the changes if you'd like to.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 it probably is better to stay as close to ecto_sql as possible. I do remember there being a reason why I split them other than "just feels" but I can't recall.

@warmwaffles

Copy link
Copy Markdown
Member

@mochaaP let me know when you are at a good point to stop for these set of changes.

@mochaaP

mochaaP commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

I'm good for now, try https://reviewable.io/reviews/elixir-sqlite/ecto_sqlite3/184 if force-pushing is noisy for reviews

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