- Notifications
You must be signed in to change notification settings - Fork928
chore: split queries.sql into files by table#762
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedMar 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #762 +/- ##==========================================+ Coverage 64.25% 65.93% +1.67%========================================== Files 199 200 +1 Lines 11861 13179 +1318 Branches 87 87 ==========================================+ Hits 7621 8689 +1068- Misses 3420 3603 +183- Partials 820 887 +67
Continue to review full report at Codecov.
|
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'm not a fan of the Go files this generates.
It misuses_ inside of Go file names, and makes the majority of files in the database package generated, which makes others harder to read.
Is it possible to output all generated into a single file like before? I love the separated SQL files, but it seems unnecessary to have separated generated Go files.
| @@ -30,6 +30,7 @@ CREATE TABLE projects ( | |||
| -- Enforces no active projects have the same name. | |||
| CREATE UNIQUE INDEX ON projects (organization_id, name) WHERE deleted = FALSE; | |||
| CREATE UNIQUE INDEX idx_projects_name_lower ON projects USING btree (lower(name)); | |||
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.
@kylecarbs we seemed to be querying these by their lowercase names so I added indexes for them as well.
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.
Ahh wise. Good change!
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.
Minor things! Neat change.
| --output ./coderd/database/query.sql | ||
| sed -i 's/@ /@/g' ./coderd/database/query.sql | ||
| fmt/sql: $(wildcard coderd/database/queries/*.sql) | ||
| # TODO: this is slightly slow |
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.
Could we use bash wait to make this faster?
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.
The biggest problem is that it has to attempt to install vianpx every time, otherwise it's pretty fast! Cannpx be called concurrently?
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.
Ohhhhhh. I say we requiresql-formatter installed globally then.
Uh oh!
There was an error while loading.Please reload this page.
No description provided.