-
Notifications
You must be signed in to change notification settings - Fork 838
chore: Cleanup coreth post-merge #4578
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
base: master
Are you sure you want to change the base?
Conversation
3ac4417 to
94d2010
Compare
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 should just be manually rewritten alongside avalanchego's next release
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.
Can we create an issue for 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.
Pull request overview
This PR performs cleanup tasks following the coreth merger into the avalanchego repository. The changes update documentation, file paths, and configurations to reflect coreth's new location within the avalanchego repository structure.
Key changes:
- Updates repository URLs and file paths from standalone coreth repository to the grafted location within avalanchego
- Removes obsolete standalone repository files (GitHub workflows, issue templates, release documentation)
- Consolidates configuration by removing duplicate files now managed at the repository root level
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vms/example/xsvm/README.md | Updates Coreth reference URL to new location within avalanchego |
| graft/coreth/scripts/dev_shell.sh | Removes obsolete development shell script |
| graft/coreth/scripts/constants.sh | Updates git directory path for commit hash retrieval |
| graft/coreth/go.mod | Simplifies Go version update guidelines by referencing root go.mod |
| graft/coreth/docs/releasing/README.md | Removes standalone release documentation |
| graft/coreth/cmd/simulator/README.md | Updates simulator directory path |
| graft/coreth/SECURITY.md | Updates releases URL to avalanchego repository |
| graft/coreth/README.md | Simplifies build instructions and removes duplicate nix setup documentation |
| graft/coreth/.gitignore | Removes file as gitignore is managed at repository root |
| graft/coreth/.github/workflows/codeql-analysis.yml | Removes duplicate workflow managed at repository root |
| graft/coreth/.github/pull_request_template.md | Removes duplicate template managed at repository root |
| graft/coreth/.github/dependabot.yml | Removes duplicate configuration managed at repository root |
| graft/coreth/.github/ISSUE_TEMPLATE/feature_request.md | Removes duplicate template managed at repository root |
| graft/coreth/.github/ISSUE_TEMPLATE/bug_report.md | Removes duplicate template managed at repository root |
| graft/coreth/.github/CONTRIBUTING.md | Removes duplicate documentation managed at repository root |
| graft/coreth/.github/CODEOWNERS | Removes duplicate file managed at repository root |
| graft/coreth/.dockerignore | Removes file no longer needed |
| go.mod | Adds reference to graft module go.mod files in version update guidelines |
| .github/dependabot.yml | Adds dependabot configuration for coreth module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1 +1 @@ | |||
| #!/usr/bin/env bash | |||
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.
Is this file still required or could the avalanchego equivalent be used instead?
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.
Most of it is still required. The only shared variable is CGO_FLAGS.
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.
Is it for more than the build script though? And why isn't that just being deleted anyway? afaik we're neither consuming nor releasing a binary artifact for coreth.
| ``` | ||
| Coreth is a dependency of AvalancheGo which is used to implement the EVM based Virtual Machine for the Avalanche C-Chain. In order to run with a local version of Coreth, users can simply build AvalancheGo from source. | ||
|
|
||
| Note: the C-Chain originally ran in a separate process from the main AvalancheGo process and communicated with it over a local gRPC connection. When this was the case, AvalancheGo's build script would download Coreth, compile it, and place the binary into the `avalanchego/build/plugins` directory. |
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.
| Note: the C-Chain originally ran in a separate process from the main AvalancheGo process and communicated with it over a local gRPC connection. When this was the case, AvalancheGo's build script would download Coreth, compile it, and place the binary into the `avalanchego/build/plugins` directory. |
| Some activities, such as collecting metrics and logs from the nodes targeted by an e2e | ||
| test run, require binary dependencies. One way of making these dependencies available is | ||
| to use a nix shell which will give access to the dependencies expected by the 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.
| Some activities, such as collecting metrics and logs from the nodes targeted by an e2e | |
| test run, require binary dependencies. One way of making these dependencies available is | |
| to use a nix shell which will give access to the dependencies expected by the test | |
| Some activities, such as collecting metrics and logs from the nodes targeted by an e2e test run, require binary dependencies. One way of making these dependencies available is to use a nix shell which will give access to the dependencies expected by the 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.
The following line you added should also not be hard broken and should be part of the same line.
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.
(sorry, can't suggested)
|
|
||
| # WARNING: this will use the most recent commit even if there are un-committed changes present | ||
| CORETH_COMMIT="$(git --git-dir="$CORETH_PATH/.git" rev-parse HEAD)" | ||
| CORETH_COMMIT="$(git --git-dir="$GOPATH/src/github.com/ava-labs/avalanchego/.git" rev-parse HEAD)" |
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.
rename to avalancego commit? seems misleading now.
Why this should be merged
I noticed some cleanup items to be done after the coreth merger, so might as well get them out of the way
How this works
Mostly deletions and file redirects
How this was tested
CI
Need to be documented in RELEASES.md?
No