-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: zk fmt sqlx-queries #533
Conversation
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.
Awesome idea! 👍 As usual, I do have some bikeshedding topics.
0f8aa7c
to
b376916
Compare
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.
LGTM, but I'd love other opinions as to whether this formatting makes sense. As some potential alternatives (given that we have a tool to unify SQL queries):
- We could use raw literal strings without indentation. IMO, they look ugly.
- We could use raw literal strings with indentation. Doesn't satisfy the "query is not recompiled regardless of surrounding formatting" criterion.
- We could extract queries into files (i.e., use
query_file
etc.). IMO, extracting queries makes it more difficult to keep the query context when reviewing code.
FWIW, I've tried updating sqlx
to the latest version (0.7), and it still doesn't transform whitespace in query literals; so, we wouldn't avoid the formatting problem by updating.
@slowli I will add this formatting to agenda to our next weekly platform team meeting |
b376916
to
6a71860
Compare
🤖 I have created a release *beep* *boop* --- ## [18.9.0](core-v18.8.0...core-v18.9.0) (2023-12-19) ### Features * Add ecadd and ecmul to the list of precompiles upon genesis ([#669](#669)) ([0be35b8](0be35b8)) * **api:** Do not return receipt if tx was not included to the batch ([#706](#706)) ([625d632](625d632)) * proto serialization/deserialization of snapshots creator objects ([#667](#667)) ([9f096a4](9f096a4)) * zk fmt sqlx-queries ([#533](#533)) ([6982343](6982343)) ### Bug Fixes * **en:** Downgrade miniblock hash equality assertion to warning ([#695](#695)) ([2ef3ec8](2ef3ec8)) ### Performance Improvements * remove unnecessary to_vec ([#702](#702)) ([c55a658](c55a658)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
What ❔
Added option to format queries using
zk fmt sqlx-queries
It uses https://github.com/sql-formatter-org/sql-formatter as a formatter
Checklist
zk fmt
andzk lint
.