-
Notifications
You must be signed in to change notification settings - Fork 316
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
Convert submission insertion and grading selection sprocs to lib #9434
Conversation
All images
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these changes to core code make me nervous, but I went through this pretty carefully and this all looks good. Let's hope our tests catch all the scenarios we care about!
VariantWithFormattedDateSchema, | ||
); | ||
|
||
const variant = await sqldb.runInTransactionAsync(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on the line, but: the group_work
parameter to makeAndInsertVariant
is now unused (and was in fact unused before - we passed it to the sproc, but the sproc did nothing with it). Consider removing to clean that up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 79ff2ab. This actually cascaded into multiple functions that also only had that parameter to pass it along. I'll do another passthrough to ensure everything is fine before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd also be fine to save this for another, smaller PR if it cascades into too many changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was 5 functions, most of them local, so not too much.
No description provided.