-
Notifications
You must be signed in to change notification settings - Fork 5
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
refactor: prepare migration to forge #32
base: forge-dev
Are you sure you want to change the base?
Conversation
18fe37f
to
81d050b
Compare
ci: restore deps in compile job ci: restore node_modules in tests job chore: format ci: debug ci: fix output name in if check ci: debug ci: debug ci: fix coveralls step chore: remove debug test: exclude some packages from coverage ci: debug ci: have slither job wait for compile ci: debug ci: debug
0c294bd
to
45dbcc1
Compare
04999d9
to
9ffc7e4
Compare
5305e64
to
13c23df
Compare
"format": "prettier -c .", | ||
"format:write": "prettier -w .", | ||
"remove:stable-version-field": "ts-node scripts/remove-stable-version-field.ts ${0} && yarn format:write", | ||
"test": "forge test", |
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 decided to remove remove:stable-version-field
and version:publish
because this is handled by the ci, I think it is better not to easily "expose" such scripts in package.json
"format:write": "prettier -w .", | ||
"remove:stable-version-field": "ts-node scripts/remove-stable-version-field.ts ${0} && yarn format:write", | ||
"test": "forge test", | ||
"version:bump": "scripts/version-bump.sh", |
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 had to make this a sh script because the repo does not use any typescript dev deps anymore
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.
coveralls report: coverage not 100%, I am especially not covering the revert on WrongSiblingNodes
(didn't figure out how to write a corresponding test 😅
I am not sure why the coveralls bot does not write comments in the PR btw.
# TODO: update, temporarily ignoring all except lean-imt | ||
no_match_coverage = "^packages/(excubiae|imt|lazy-imt|lazytower)" |
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.
update this as we rewrite more tests in solidity
|
||
rm -fr "packages" | ||
|
||
git commit -am "$latest_main_commit_msg" |
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.
this script is executed in the release
workflow...
which is triggered on tags push events on main...
Tags are created with yarn version:bump
...
Which always creates a chore(pkg): v(version)
commit...
So on the forge
branch we will only have a clean succession of chore(pkg): v(version)
commit messages that mirrors our npm release workflow, making it easy for devs to checkout/select a specific contracts package version even if they install with forge install
Closes #30
This prepares the migration to forge as solidity development toolchain.
This PR makes the following changes:
forge
branch for installation with forge:forge install privacy-scaling-explorations/zk-kit.solidity@forge
. Asforge install
uses submodules and all our contracts are in the same repo, it is not possible to selectively install a zk kit contract package to install eg onlyexcubiae
withforge install
, the whole repo or a single branch of the repo has to be cloned.lean-imt
as a start: I suggest to let package authors rewrite their tests themselves later (see dx(excubiae): migrate toforge
#26 dx(imt): migrate toforge
#27 dx(lazy-imt): migrate toforge
#28 dx(lazytower): migrate toforge
#29)