-
Notifications
You must be signed in to change notification settings - Fork 9
Fixing #11 (enum name harmony) and #12 (pass down transaction) #13
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
Fixing #11 (enum name harmony) and #12 (pass down transaction) #13
Conversation
Temporary until abelosorio#13 gets merged
I've taken the liberty of publishing your changeset under |
Great news dude :) |
(I'll close issues when merged into release ?) |
NB: One thing to note is that I'm using a not-very-documented sequelize Feature : when you pass a NB2: I'm not intentionally fail the parent transaction also
intentionally handle the error, and resolve parent transaction
|
Hi @cyrilchapon @TheOptimisticFactory, have you guys tested this PR? Currently, I don't have time for this project, so I'm relying on you for this. Let me know if this is fully tested and I'll merge it. |
Hey @abelosorio I wrote a couple of unit tests, but that's the kind of features I usually test with "side effects tests" (against a real database instead of a mocked one). |
To be perfectly honest I would not be 100% confident deploying it on production (though, I did). I think this is so specific that it needs to be unit tested agasint a real database |
To override explicitly null / 0 / "" passed values, but this is overkill indeed |
Yeah, I think so as well. Can you remove it so I can merge this? Thanks |
Fixing #11
Fixing #12
Transaction passing down require additional manual testing