chore: add local dependencies#11
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
CoW DAO has been using pnpm for most modern repos. Examples:
- https://github.com/cowprotocol/cowswap/blob/develop/pnpm-lock.yaml
- https://github.com/cowprotocol/deployments/blob/main/pnpm-lock.yaml
(having used both pnpm and npm myself and migrated, I can confirm pnpm is the superior manager 😆 )
There was a problem hiding this comment.
I don't particularly mind either way but I agree pnpm is nicer.
| @@ -0,0 +1 @@ | |||
| slither-analyzer==0.11.5 | |||
There was a problem hiding this comment.
ah jeez is slither actually in python? lame.
I'd almost prefer using the docker release (Docker use is widespread in this org) https://github.com/crytic/slither#using-docker to avoid having this repo depend on foundry, node, and python. But I agree we need slither.
fedgiac
left a comment
fedgiac
left a comment
There was a problem hiding this comment.
Looks good!
The local commands are a bit long, but this will be solved with the makefile.
| - Unckeck "Allow merge commits" option | ||
| - Check "Allow auto-merge" option | ||
| - [ ] Run `forge install` to install the dependencies. This will create a new `foundry.lock` file which you should commit to the project | ||
| - [ ] Set up [Local tooling](#local-tooling) so Solhint and Slither use the pinned project versions |
There was a problem hiding this comment.
Does it make sense to ask here to update the project versions? Ideally we'd be doing it here automatically with some tool like Dependabot, so maybe it's fine to ignore this and bump version numbers automatically later.
| "license": "UNLICENSED", | ||
| "devDependencies": { | ||
| "solhint": "6.0.3" | ||
| } |
There was a problem hiding this comment.
Just came to my mind: we can reduce the risk of supply-chain attacks by not installing packages that have been released, say, less than 2 days ago. This gives a new package 2 days to be scrutinized by the community before we install it.
This config parameter has different names based on the chosen NPM manager. For example, for pnpm it's minimumReleaseAge.
3 participants
Description
Add local pinned JS and Python dependencies under
dev/.Context
This gives the template local tool versions instead of asking developers to install Solhint or Slither globally.
Out of Scope
This PR only adds the dependency setup. It does not add Solhint config, Slither config, Justfile commands, CI, or hooks.
Testing Instructions
npm install --prefix dev.python -m venv dev/.venv.dev/.venv/bin/pip install -r dev/requirements.txt.dev/node_modules/.bin/solhint --version.dev/.venv/bin/slither --version.