Skip to content

Remove Foundry NPM package and switch to local installation#4

Merged
igorroncevic merged 4 commits into
mainfrom
feat/pin-foundry-toolchain
May 19, 2026
Merged

Remove Foundry NPM package and switch to local installation#4
igorroncevic merged 4 commits into
mainfrom
feat/pin-foundry-toolchain

Conversation

@igorroncevic
Copy link
Copy Markdown
Collaborator

Summary

  • remove Foundry npm packages from dev dependencies and lockfile
  • run Foundry commands from the local PATH instead of npm-provided binaries
  • install Foundry v1.7.0 in the shared CI setup action and document local v1.7.0 setup
  • remove direct Foundry passthrough recipes from just

Test Plan

  • forge --version
  • pnpm --dir dev install --lockfile-only --offline
  • just build
  • just lint
  • just test
  • uv sync --project dev --locked
  • just slither
  • just coverage-check

@igorroncevic igorroncevic requested a review from a team May 19, 2026 08:18
@igorroncevic
Copy link
Copy Markdown
Collaborator Author

PR in template repo: cowprotocol/contracts-template#26

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the project from using local npm-wrapped Foundry binaries to the official Foundry toolchain by removing the @foundry-rs dependencies and updating the Justfile and CI configuration. Feedback indicates that the version v1.7.0 specified in the GitHub Action and README is incorrect, as it refers to the deprecated npm wrapper versioning rather than valid official Foundry release tags. The documentation and CI setup need to be updated with a correct version or commit hash to prevent installation failures.

Comment thread .github/actions/setup/action.yml Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
@igorroncevic igorroncevic changed the title Use pinned local Foundry toolchain Remove Foundry NPM package and switch to local installation May 19, 2026
Comment thread .github/actions/setup/action.yml Outdated
Comment thread README.md Outdated
@igorroncevic igorroncevic merged commit 9a43d63 into main May 19, 2026
6 checks passed
@igorroncevic igorroncevic deleted the feat/pin-foundry-toolchain branch May 19, 2026 10:36
Copy link
Copy Markdown

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, im a bit lot on the motivation of this PR. I believe could provide more context

In the past you did one to install it using NPM , and cause NPM is still a dependency seemed a nice way to ensure we all use the same version.

Not saying is wrong, but i didn't get the motivation, plus the reason the README now has some commands in they lifecicle using just and some use forge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants