-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
enable style:usages for stdlib tests [backport: 1.6] #19715
Conversation
@@ -150,25 +150,25 @@ proc tryExec*(db: DbConn, query: SqlQuery, args: varargs[string, `$`]): bool {. | |||
tags: [ReadDbEffect, WriteDbEffect].} = | |||
## tries to execute the query and returns true if successful, false otherwise. | |||
var q = dbFormat(query, args) | |||
return mysql.realQuery(PMySQL db, q, q.len) == 0'i32 | |||
return mysql.real_query(PMySQL db, q, q.len) == 0'i32 |
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 would like to use camelCase, however the whole file of mysql.nim uses snake_case. So I decide to live with it now.
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.
what we've done in such cases is to separate the pure C API into a private module that does nothing else than importc
, then add a wrapper on top that makes the wrapper convenient to use from nim - this works pretty well because it also separates the responsibilities of the various modules along a clean line
See my remarks. Otherwise it's excellent. |
* enable style:usages for stdlib tests * freeAddrInfo * more tests * importc * bufSize * fix more * => parseSql and renderSql (cherry picked from commit 98cebad)
* enable style:usages for stdlib tests * freeAddrInfo * more tests * importc * bufSize * fix more * => parseSql and renderSql
No description provided.