-
Notifications
You must be signed in to change notification settings - Fork 157
Horizon upgrades in preparation for contract PRs #1186
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
fix: fix and upgrade Husky pre-commit, and add dev container config
More robust handling of unusual filenames. Co-authored-by: Copilot <[email protected]>
Without a container name will automatically be picked based on dir
Dev container fixes and updates
Migrated eslint-graph-config and solhint-graph-config to be configuration files in repo root that are inherited from for packages. It is presumed we do not need to maintain these config packages for external dependencies, and they have been deleted. Upgraded linting engines (ESLint, Solhint, Prettier) and added linting of Markdown, JSON, and YAML. Upgraded to Node version 20 (from 18). Might not work anymore when using Node version 18. Consistently made Prettier the last to process during linting, upgrading Prettier config to match ESLint config and avoid conflicts. Changed import ordering to be automated (by running yarn lint) using eslint-plugin-simple-import-sort. Also sorts exports. Dropped no-secrets ESLint plugin due to version conflicts and low signal to noise ratio. Upgraded Yarn and resolved dependency version clashes. Upgraded and simplified Husky pre-commit behaviour. Currently using repo root .gitignore as single source for lint files to ignore. This is imperfect, and might need to be subsequently adjusted and revisited. Attempted to preserve Lint/Prettier config parameters, however I think import and export sorting have changed, and in some cases dangling commas will be added where they were previously removed (an improvement IMO). Upgraded linting engines mean more issues are detected, and likely some changes in behaviour. Added incremental and cached linting and building in various places. Bumping versions of contracts and token-distribution as the TS interfaces are not backwards compatible, and minor of other packages as should be considered a different version now. GitHub workflow to run lints.
- Upgrade to pnpm, improving package management and converging with Horizon. - Restructure packages to remove dependency cycles. Instead of the sdk depending on contracts and contracts on sdk, contracts has child packages contracts-test and contracts-task that depend on sdk but that sdk does not depend on. These child packages depend on sdk, while contracts does not. A common package has also been created for shared dependencies of contract packages. There are now no dependency cycles, no SKIP_LOAD compiles, and dynamic loading has been converted to static imports. - Numerous lint issues have been fixed. ESLint upgraded to v9 and natspec-smells linting added. - GH workflows have been simplified. One for linting, one for build, test, and coverage test. Workflows use pnpm to find build, test, and test:coverage targets rather than needing separate configuration or per package workflows.
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
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.
Pull Request Overview
This PR updates various configuration files and GitHub workflows to streamline the Horizon tooling in preparation for contract PRs. Key changes include new linting configurations (.yamllint, .solhint.json, prettier, markdownlint), simplified Husky hooks, and multiple workflow updates such as a change in the contracts artifacts directory and adjustments in Node.js version.
Reviewed Changes
Copilot reviewed 390 out of 390 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
.yamllint | New YAML linting configuration with custom rules |
.solhint.json | New Solidity linting settings |
.prettierignore, .markdownlintignore, .markdownlint.json | Updated ignore and configuration files for Prettier and Markdownlint |
.husky/pre-commit, .husky/commit-msg | Simplified hook commands |
.github/workflows/* | Updated and removed several GitHub workflows to align with new tooling, including a change in the artifacts directory for contracts |
.github/actions/setup/action.yml | Adjusted system dependency installation and updated Node.js version from 22 to 20 |
Comments suppressed due to low confidence (2)
.github/actions/setup/action.yml:20
- Review the change in Node.js version from 22 to 20 to verify that the downgraded version meets all project dependencies and tooling requirements.
node-version: 20
.github/workflows/verifydeployed.yml:43
- Confirm that the change from 'build' to 'artifacts' for contract artifacts is intentional and that all references to the build output have been updated consistently.
packages/contracts/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.
looks good, left a few minor comments.
Horizon upgrades in preparation for contract PRs
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## horizon #1186 +/- ##
===========================================
- Coverage 92.56% 82.84% -9.72%
===========================================
Files 47 47
Lines 2435 2093 -342
Branches 440 620 +180
===========================================
- Hits 2254 1734 -520
- Misses 181 359 +178
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Converging Horizon tooling and updates per PR#1184.