-
Notifications
You must be signed in to change notification settings - Fork 293
[TKN-682] Commit Codama-generated clients (TS/Rust) and add verify-generated CI #1073
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
Conversation
…centralize Codama via resolutions and add Nx generate targets
…enerated exceptions
| - '.github/workflows/**' | ||
|
|
||
| jobs: | ||
| verify-generated: |
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.
Maybe we can just add this to the checks.yml file?
.gitignore
Outdated
| solana-release | ||
| rust-sdk/integration/**/Cargo.lock | ||
| ts-sdk/integration/**/Cargo.lock | ||
| !/ts-sdk/client/src/generated/ |
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.
Are there any other */generated files? We might just remove the whole entry
.prettierignore
Outdated
| @@ -0,0 +1,2 @@ | |||
| /ts-sdk/client/src/generated/ | |||
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 think as part of the generate script we should run formatter. Now it is publishing unformatted code
package.json
Outdated
| "lint": "nx run-many --target lint --projects", | ||
| "clean": "nx reset && nx run-many --target clean --projects" | ||
| "clean": "nx reset && nx run-many --target clean --projects", | ||
| "generate:idl": "anchor build", |
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 think you should instead of adding the scripts here use nx to delegate the generate command to each respective sub-project.
package.json
Outdated
| "tsup": "^8.4.0", | ||
| "vitest": "^3.0.9" | ||
| }, | ||
| "resolutions": { |
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.
Why do we need resolutions 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 idea is that the generator templates/formatting can change across releases, causing diffs without any program changes. The verify-generated check could fail on incidental upstream changes. I added resolutions to centralize the exact versions.
rust-sdk/client/package.json
Outdated
| "scripts": { | ||
| "build": "node ./codama.js && cargo build", | ||
| "generate": "node ./codama.js", | ||
| "format:generated": "yarn workspace @orca-so/whirlpools-lint prettier --write ../../rust-sdk/client/src/generated", |
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 issue with prettier is that it infers most of the rules based on the existing code. You might have to run the normal format command. Also why are we running prettier on rust code? 👀
ts-sdk/client/package.json
Outdated
| "scripts": { | ||
| "build": "node ./codama.js && tsup src/index.ts --format cjs,esm --dts --sourcemap", | ||
| "generate": "node ./codama.js", | ||
| "format:generated": "yarn workspace @orca-so/whirlpools-lint prettier --write ../../ts-sdk/client/src/generated", |
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.
Same thing with this one. I uguess just running the full formatter would work better
package.json
Outdated
| "generate:ts": "nx run @orca-so/whirlpools-client:generate || yarn --cwd ts-sdk/client run generate", | ||
| "generate:rs": "nx run @orca-so/whirlpools-rust-client:generate || yarn --cwd rust-sdk/client run generate", | ||
| "generate:clients": "yarn generate:ts && yarn generate:rs" | ||
| "generate:clients": "yarn generate:ts && yarn generate:rs && yarn format:generated", |
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.
These should be using nx and not defined manually as a script
|
|
||
| "@typescript-eslint/consistent-type-imports": "error", | ||
| "@typescript-eslint/consistent-type-exports": "error", | ||
| "@typescript-eslint/no-empty-object-type": "off", |
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 generated code has a lot of empty objects, so added this rule
wjthieme
left a comment
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.
🚢
Commit Codama-generated sources:
ts-sdk/client/src/generatedrust-sdk/client/src/generatedAdd CI workflow
verify-generated.yml:./.github/actions/anchor(Rust v1.84.0, Node v22.8.0, Solana v1.17.25, Anchor v0.29.0)anchor build,yarn generate:clients, and fails on diffs in generated dirsAdd generation scripts and Nx targets:
generate:idl,generate:ts,generate:rs,generate:clientsgeneratescriptCentralize Codama versions via root
resolutionsUpdate
.gitignoreto ensure generated dirs are trackedAdd
.prettierignoreto exclude lint checks on generated code