Fix sqlite pragma issue

moigagoo / norm

A Nim ORM for SQLite and Postgres

https://norm.nim.town

MIT License

378 stars 34 forks source link

Fix sqlite pragma issue #185

Closed PhilippMDoerner closed 1 year ago

PhilippMDoerner commented 1 year ago

Implements #184 in the following scope:

Enables Foreign key Pragma for all connections created via getDb()
Modifies "getDbProc" when creating an sqlite Pool to also enable the Foreign Key Pragma
Adds docs educating about the situation in case the user wants to have that behaviour on a connection they created themselves
Adds tests checking that a DB Error correctly occurs if a Foreign-key Constraint Violation occurs
Updated changelog
Added equal tests to Postgres so the tests are symmetrical and we're making sure that both cover the exact same scope of behaviour

moigagoo commented 1 year ago

@PhilippMDoerner I'm happy with the PR but there's just one thing I'd like you to change. It's something so minor I'm feeling awkward even bringing this up. But could you please spell “Norm” and not “norm” in your copy? :-)

PhilippMDoerner commented 1 year ago

No worries. By "in your copy" you mean in the book docs? Or just generally go over the project and replace Norm with norm where its not at the start of a sentence?

moigagoo commented 1 year ago

No worries. By "in your copy" you mean in the book docs? Or just generally go over the project and replace Norm with norm where its not at the start of a sentence?

I meant all places where it's not code. I.e., docs, book, comments.

PhilippMDoerner commented 1 year ago

Done! I did leave "Norm" in capital letter at the start of sentences for grammatical correctness there (at least IIRC if it's at the start of a sentence it's always capitalized, no exceptions). So there's still sentences like this:

Norm's `getDb` proc lets you create a DB connection using `DB_HOST`, `DB_USER`, `DB_PASS`, and `DB_NAME` environment variables:

If you want me to change those as well, give a shout, other than that the most recent commit should've taken care of it.

moigagoo commented 1 year ago

@PhilippMDoerner https://github.com/moigagoo/norm/pull/185/commits/fe4f660026a2624459b681696c00d85d810bad06 It's the other way around :-)

Capitalized “Norm” is the right spelling. I've noticed that you tend to use “norm” instead.

PhilippMDoerner commented 1 year ago

Wow I'm tired xD

Alrighty, one sec

PhilippMDoerner commented 1 year ago

Okay, here we go, attempt number 2 of 2 possible solutions