-
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
chore: rename prisma-fmt-wasm to prisma-schema-wasm #20045
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
CodSpeed Performance ReportMerging #20045 will degrade performances by 11.38%Comparing Summary
Benchmarks breakdown
|
Note: ignore the "The system cannot find the path specified." for Windows run, it looks like it's flaky, only happens in some runs. |
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 seems that both the CI and the codebase are handled! Nice, approving with a small nit that doesn't block merges.
describe('format wasm', () => { | ||
describe('schema wasm', () => { |
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.
doesn't this refer to the action of formatting?
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.
Yes, here actually a better name would be "format" but actually these tests are now duplicates of the ones below, and we could clean up there, though not urgent at all.
@@ -36,6 +36,7 @@ export enum ErrorArea { | |||
PHOTON_STUDIO = 'PHOTON_STUDIO', | |||
// Unused since 4.9.0 and now using `LIFT_CLI` | |||
INTROSPECTION_CLI = 'INTROSPECTION_CLI', | |||
// Note: prisma-fmt-wasm was renamed to prisma-schema-wasm in 5.0.0 |
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 does this refer to here?
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 error area is something used in our internal error reporting dashboard to categorise Rust panics' "topic areas". It mirrors this ErrorArea
enum here.
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.
Hm, so this defines FMT_CLI
further?
Still confuses me, as the wording that I commented on does not really make this clear.
It sounds like it would comment on something talking about prisma-fmt-wasm
or prisma-schema-wasm
, but that is not to be found in this file.
Part of prisma/prisma-engines#4033