-
Notifications
You must be signed in to change notification settings - Fork 10
refactor: biome lint, fmt commands, and improve turbo scripts #274
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
andrew-fleming
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.
@0xisk changes look good! I left a few comments
turbo.json
Outdated
| ], | ||
| "outputLogs": "new-only", | ||
| "outputs": [ | ||
| "src/artifacts/**" |
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.
| "src/artifacts/**" | |
| "artifacts/**" |
artifacts are generated at the same level as the src and package.json e.g. access/artifacts
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.
If you agree, same with the regular compact task
| "compact:fast": "turbo run compact:fast", | ||
| "compact:version": "turbo run compact -- --version", | ||
| "compact:language-version": "turbo run compact -- --language-version", |
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 version and language-version scripts are returning 2.6.0 (the version of turbo)
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 actually outputs 0.26.0 on my end, which is right:
pnpm compact:language-version --fast ✔ ▼ 22.18.0
> @openzeppelin/midnight-apps@1.0.0 compact:language-version /home/iskander/Projects/midnight-dapps
> turbo run compact -- --language-version --fast
╭──────────────────────────────────────────────────────────────────────────╮
│ │
│ Update available v2.6.0 ≫ v2.7.1 │
│ Changelog: https://github.com/vercel/turborepo/releases/tag/v2.7.1 │
│ Run "pnpm dlx @turbo/codemod@latest update" to update │
│ │
│ Follow @turborepo for updates: https://x.com/turborepo │
╰──────────────────────────────────────────────────────────────────────────╯
turbo 2.6.0
• Packages in scope: @openzeppelin/midnight-apps-access, @openzeppelin/midnight-apps-biome-config, @openzeppelin/midnight-apps-compact, @openzeppelin/midnight-apps-compact-std, @openzeppelin/midnight-apps-logger, @openzeppelin/midnight-apps-lunarswap-ui, @openzeppelin/midnight-apps-math, @openzeppelin/midnight-apps-shielded-token, @openzeppelin/midnight-apps-structs
• Running compact in 9 packages
• Remote caching disabled
@openzeppelin/midnight-apps-structs:compact: cache miss, executing 761c3c139239f45a
@openzeppelin/midnight-apps-structs:compact:
@openzeppelin/midnight-apps-compact-std:compact: cache miss, executing 3b12c0a05b2ad858
@openzeppelin/midnight-apps-compact-std:compact:
@openzeppelin/midnight-apps-shielded-token:compact: cache miss, executing 4644e56854dc78b4
@openzeppelin/midnight-apps-shielded-token:compact:
@openzeppelin/midnight-apps-access:compact: cache miss, executing ad218574219579ef
@openzeppelin/midnight-apps-access:compact:
@openzeppelin/midnight-apps-math:compact: cache miss, executing d7ee71a00a2f9a84
@openzeppelin/midnight-apps-math:compact:
@openzeppelin/midnight-apps-structs:compact:
@openzeppelin/midnight-apps-structs:compact: > @openzeppelin/midnight-apps-structs@1.0.0-alpha.1 compact /home/iskander/Projects/midnight-dapps/contracts/structs
@openzeppelin/midnight-apps-structs:compact: > pnpm exec compact-compiler --language-version --fast
@openzeppelin/midnight-apps-structs:compact:
@openzeppelin/midnight-apps-compact-std:compact:
@openzeppelin/midnight-apps-compact-std:compact: > @openzeppelin/midnight-apps-compact-std@1.0.0-alpha.1 compact /home/iskander/Projects/midnight-dapps/contracts/compact-std
@openzeppelin/midnight-apps-compact-std:compact: > pnpm exec compact-compiler --language-version --fast
@openzeppelin/midnight-apps-compact-std:compact:
@openzeppelin/midnight-apps-math:compact:
@openzeppelin/midnight-apps-math:compact: > @openzeppelin/midnight-apps-math@1.0.0-alpha.1 compact /home/iskander/Projects/midnight-dapps/contracts/math
@openzeppelin/midnight-apps-math:compact: > pnpm exec compact-compiler --language-version --fast
@openzeppelin/midnight-apps-math:compact:
@openzeppelin/midnight-apps-access:compact:
@openzeppelin/midnight-apps-access:compact: > @openzeppelin/midnight-apps-access@1.0.0-alpha.1 compact /home/iskander/Projects/midnight-dapps/contracts/access
@openzeppelin/midnight-apps-access:compact: > pnpm exec compact-compiler --language-version --fast
@openzeppelin/midnight-apps-access:compact:
@openzeppelin/midnight-apps-shielded-token:compact:
@openzeppelin/midnight-apps-shielded-token:compact: > @openzeppelin/midnight-apps-shielded-token@1.0.0-alpha.1 compact /home/iskander/Projects/midnight-dapps/contracts/shielded-token
@openzeppelin/midnight-apps-shielded-token:compact: > pnpm exec compact-compiler --language-version --fast
@openzeppelin/midnight-apps-shielded-token:compact:
ℹ [COMPILE] Compact compiler startedcompact:
ℹ [COMPILE] Compact compiler startedstd:compact:
ℹ [COMPILE] Compact compiler startedompact:
ℹ [COMPILE] Compact compiler startedpact:
ℹ [COMPILE] Compact compiler started-token:compact:
ℹ [COMPILE] Compact developer tools: compact 0.3.0
ℹ [COMPILE] Compact toolchain: 0.26.0ompact:
ℹ [COMPILE] Using toolchain version: 0.26.0:
ℹ [COMPILE] Found 2 .compact file(s) to compile
ℹ [COMPILE] Compact developer tools: compact 0.3.0
ℹ [COMPILE] Compact toolchain: 0.26.0td:compact:
ℹ [COMPILE] Using toolchain version: 0.26.0pact:
ℹ [COMPILE] Found 1 .compact file(s) to compile:
ℹ [COMPILE] Compact developer tools: compact 0.3.0
ℹ [COMPILE] Compact toolchain: 0.26.0act:
ℹ [COMPILE] Using toolchain version: 0.26.0
ℹ [COMPILE] Found 16 .compact file(s) to compile
ℹ [COMPILE] Compact developer tools: compact 0.3.0
ℹ [COMPILE] Compact toolchain: 0.26.0mpact:
ℹ [COMPILE] Using toolchain version: 0.26.0
ℹ [COMPILE] Found 3 .compact file(s) to compile
ℹ [COMPILE] Compact developer tools: compact 0.3.0:
ℹ [COMPILE] Compact toolchain: 0.26.0token:compact:
ℹ [COMPILE] Using toolchain version: 0.26.0compact:
ℹ [COMPILE] Found 3 .compact file(s) to compileact:
✔ [COMPILE] [1/2] Compiled Queue.compact
@openzeppelin/midnight-apps-structs:compact:
✔ [COMPILE] [1/1] Compiled Index.compact
@openzeppelin/midnight-apps-compact-std:compact:
✔ [COMPILE] [1/16] Compiled Bytes32.compact
@openzeppelin/midnight-apps-math:compact:
@openzeppelin/midnight-apps-compact-std:compact:
✔ [COMPILE] [1/3] Compiled AccessControl.compact
@openzeppelin/midnight-apps-access:compact:
✔ [COMPILE] [1/3] Compiled ShieldedFungibleToken.compact
@openzeppelin/midnight-apps-shielded-token:compact:
✔ [COMPILE] [2/2] Compiled test/Queue.mock.compact
@openzeppelin/midnight-apps-structs:compact:
@openzeppelin/midnight-apps-structs:compact:
✔ [COMPILE] [2/16] Compiled Field254.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [2/3] Compiled Index.compact
@openzeppelin/midnight-apps-access:compact:
✔ [COMPILE] [2/3] Compiled openzeppelin/ShieldedERC20.compact
@openzeppelin/midnight-apps-shielded-token:compact:
✔ [COMPILE] [3/16] Compiled Index.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [3/3] Compiled test/AccessControl.mock.compact
@openzeppelin/midnight-apps-access:compact:
@openzeppelin/midnight-apps-access:compact:
✔ [COMPILE] [3/3] Compiled openzeppelin/Utils.compact
@openzeppelin/midnight-apps-shielded-token:compact:
@openzeppelin/midnight-apps-shielded-token:compact:
✔ [COMPILE] [4/16] Compiled Max.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [5/16] Compiled Uint128.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [6/16] Compiled Uint256.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [7/16] Compiled Uint64.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [8/16] Compiled interfaces/IUint128.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [9/16] Compiled interfaces/IUint256.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [10/16] Compiled interfaces/IUint64.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [11/16] Compiled test/Bytes32.mock.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [12/16] Compiled test/Field254.mock.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [13/16] Compiled test/Max.mock.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [14/16] Compiled test/Uint128.mock.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [15/16] Compiled test/Uint256.mock.compact
@openzeppelin/midnight-apps-math:compact:
✔ [COMPILE] [16/16] Compiled test/Uint64.mock.compact
@openzeppelin/midnight-apps-math:compact:
@openzeppelin/midnight-apps-math:compact:
Tasks: 5 successful, 5 total
Cached: 0 cached, 5 total
Time: 4.323s
...Finishing writing to cache... WARNING no output files found for task @openzeppelin/midnight-apps-access#compact. Please check your `outputs` key in `turbo.json`
WARNING no output files found for task @openzeppelin/midnight-apps-math#compact. Please check your `outputs` key in `turbo.json`
WARNING no output files found for task @openzeppelin/midnight-apps-shielded-token#compact. Please check your `outputs` key in `turbo.json`
WARNING no output files found for task @openzeppelin/midnight-apps-structs#compact. Please check your `outputs` key in `turbo.json`
~/Pr/midnight-dapps refactor/refactor-ci *36 !1 ?2 ✔ 5s ▼ 22.18.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.
Right, but this is just compiling the contracts which indirectly provides the version output, no? It looks like the arguments aren't being passed correctly bc this is the output from interacting with the compact tools cli directly:
$ compact --version
compact 0.2.0
$ compact compile --version
0.26.0
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
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.
Right, but this is just compiling the contracts which indirectly provides the version output, no? It looks like the arguments aren't being passed correctly bc this is the output from interacting with the compact tools cli directly:
$ compact --version
compact 0.2.0
$ compact compile --version
0.26.0
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
This doesn't have to be a focus here. We can tackle it elsewhere bc it's not really significant
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.
Right, that's fixed in the compact:version. Could you confirm?
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.
compact:version outputs the language version—which is better, but should this be compact:language-version and the former is the compact dev tools version? I'm also wondering if this is needed bc it's executing the command from the user's ctx. Unless we're pinning versions that are injected automagically (e.g. +0.27.0-rc.1), is there value in this?
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.
Looking at those three options again.
~ compact --version ✔ 22.18.0
compact 0.3.0
~ compact compile --version ✔ 22.18.0
0.26.0
~ compact compile --language-version ✔ 22.18.0
0.18.0
~
I think actually there is no value from them, I think you are right. The reason I think I got confused before with the last command I thought it returns the pragma version of the contract and then we can verify it, but that is not the case it just returns the language pragma version of that compiler version, regardless of the version you use on your contracts. So I think we should remove them. Thank you for looking into this.
Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Signed-off-by: 0xisk <0xisk@proton.me>
Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Signed-off-by: 0xisk <0xisk@proton.me>
Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Signed-off-by: 0xisk <0xisk@proton.me>
Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Signed-off-by: 0xisk <0xisk@proton.me>
andrew-fleming
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.
Improvements look good! I left a couple comments that we can ignore for now. Let's just add .next/** to outputs in build:fast and this is good to go
| "compact:fast": "turbo run compact:fast", | ||
| "compact:version": "turbo run compact -- --version", | ||
| "compact:language-version": "turbo run compact -- --language-version", |
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.
Right, but this is just compiling the contracts which indirectly provides the version output, no? It looks like the arguments aren't being passed correctly bc this is the output from interacting with the compact tools cli directly:
$ compact --version
compact 0.2.0
$ compact compile --version
0.26.0
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
| "compact:fast": "turbo run compact:fast", | ||
| "compact:version": "turbo run compact -- --version", | ||
| "compact:language-version": "turbo run compact -- --language-version", |
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.
Right, but this is just compiling the contracts which indirectly provides the version output, no? It looks like the arguments aren't being passed correctly bc this is the output from interacting with the compact tools cli directly:
$ compact --version
compact 0.2.0
$ compact compile --version
0.26.0
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
Correct me if I'm wrong, but this is the idea of the script (which is useful when we pin the version of the compiler like we do with 0.27.0-rc.1)
This doesn't have to be a focus here. We can tackle it elsewhere bc it's not really significant
andrew-fleming
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.
The open discussion isn't worth stalling this PR IMO. We can discuss and/or fix it later. LGTM!
Types of changes
What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an
xin the boxes that applyFixes #???
PR Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...