-
Notifications
You must be signed in to change notification settings - Fork 840
build: bundle subnet-evm releasing into releasing workflow #4675
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?
build: bundle subnet-evm releasing into releasing workflow #4675
Conversation
20a052b to
baa013a
Compare
RELEASING_README.md
Outdated
| @@ -0,0 +1,428 @@ | |||
| # AvalancheGo Release Guide | |||
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 is a combination of the two following old files, edited for clarity, with some monorepo specific details added
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 think most of us will struggle to provide substantive review of what is presented here. There is too much content and hard to see what is changing despite the links provided above. I think it would be preferable to ensure a more reviewable structure e.g.
- Start with one of the docs that has the meat of the content
- Incrementally add content so that reviewers can see what is changing
- Iterate until this final form.
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.
Would you like me to do the documentation in a separate PR? Because of the similarity score stuff, the GitHub UI won't show it as a file move in its end state. (does gerrit have a fix 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.
I think another PR makes sense. Maybe rename the file in this PR and then modify it in a follow-on? The main thing to avoid is moving and substantially changing a big file in the same PR.
Gerrit has this issue too, but a gerrit change (PR) targets a single commit that can have multiple versions. Updating a commit involves modifying it and force pushing, each new push creates a new version, and a given review is tied to a specific version.
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.
Ah that's very interesting! tying reviews to versions is quite cool (vs. github it's either out of date or not). Anyway, I've just done the file name change, and will doe the content in a standalone PR.
| build: | ||
| desc: Compile Subnet EVM binary with git commit and static linking flags | ||
| cmd: ./scripts/build.sh # ci.yml | ||
| cmd: ./scripts/build.sh {{.CLI_ARGS}} # ci.yml |
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 okay to do in the taskfile?
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 don't see why you need this at all
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 matches that other scripts do -- the build script accepts an optional custom binary path as an argument, so someone could change the location it's built to.
baa013a to
3691fa9
Compare
alarso16
left a comment
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 think this is on the right track (besides all the unrelated changes), but I don't think we need to decide all this before we merge the previous 5 PRs
| build: | ||
| desc: Compile Subnet EVM binary with git commit and static linking flags | ||
| cmd: ./scripts/build.sh # ci.yml | ||
| cmd: ./scripts/build.sh {{.CLI_ARGS}} # ci.yml |
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 don't see why you need this at all
RELEASING_README.md
Outdated
| git push origin "$VERSION_RC" | ||
| ``` | ||
|
|
||
| ### 6. Test the Release Candidate |
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.
A lot of these details are internal to ava labs. Maybe this belongs in the eng-resources repository for testing and deployment
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.
You had requested it be rewritten in #4658. Would you prefer we delete this entirely from AvalancheGo and have the content elsewhere?
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 is all out of scope of this PR. Seems more intuitive to me to wait and do a wider cleanup after repos are merged, but file deletions are clear
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 don't think it's out of scope -- all the workflow deletions reflect moving code to the top level workflows folder to do a release (but GitHub won't show a move because both, sometimes nearly identical files already exist) Nothing else inside of .github was deleted because that would be out of scope and for another PR.
301e068 to
c594668
Compare
3691fa9 to
9a28aff
Compare
0f24f7c to
4c33d57
Compare
54607bc to
3959ec8
Compare
082b7ba to
b0f4586
Compare
…/avalanchego/graft/subnet-evm Rewrites all Go import statements from external package github.com/ava-labs/subnet-evm to internal graft subdirectory github.com/ava-labs/avalanchego/graft/subnet-evm.
…/avalanchego/graft/subnet-evm Rewrites all Go import statements from external package github.com/ava-labs/subnet-evm to internal graft subdirectory github.com/ava-labs/avalanchego/graft/subnet-evm.
| @@ -1,20 +0,0 @@ | |||
| name: Bench | |||
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 is just some metrics, and does not need to be a part of AvalancheGo CI imo -- if you want this, you can just run it yourself and see the results.
| @@ -1,72 +0,0 @@ | |||
| # For most projects, this workflow file will not need changing; you simply need | |||
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 is covered by the root file.
| @@ -1,19 +0,0 @@ | |||
| #!/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.
This was seemingly never used, even in subnet-evm. It was only mentioned in tests/fixture/tmpnet/README.md but that's documentation, and the information seemed false.
| @@ -1,32 +0,0 @@ | |||
| name: Publish Antithesis Images | |||
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 has been integrated with avalanchego
| @@ -1,48 +0,0 @@ | |||
| name: Publish Docker Image | |||
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 has been integrated with avalanchego
| @@ -1,58 +0,0 @@ | |||
| name: 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.
no longer necessary
| @@ -1,45 +0,0 @@ | |||
| version: 2 | |||
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.
no longer necessary
|
seem to work! |
RELEASING_README.md
Outdated
| @@ -0,0 +1,428 @@ | |||
| # AvalancheGo Release Guide | |||
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 think most of us will struggle to provide substantive review of what is presented here. There is too much content and hard to see what is changing despite the links provided above. I think it would be preferable to ensure a more reviewable structure e.g.
- Start with one of the docs that has the meat of the content
- Incrementally add content so that reviewers can see what is changing
- Iterate until this final form.
RELEASING_README.md
Outdated
| @@ -0,0 +1,428 @@ | |||
| # AvalancheGo Release Guide | |||
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 think another PR makes sense. Maybe rename the file in this PR and then modify it in a follow-on? The main thing to avoid is moving and substantially changing a big file in the same PR.
Gerrit has this issue too, but a gerrit change (PR) targets a single commit that can have multiple versions. Updating a commit involves modifying it and force pushing, each new push creates a new version, and a given review is tied to a specific version.
Why this should be merged
This PR does the legwork to ensure that subnet-evm is also built and provided when AvalancheGo is tagged. As planned:
How this works
I just added steps to build the subnet-evm stuff right next to the AvalancheGo steps, largely by copying and pasting / then tweaking the values.
How this was tested
CI
Need to be documented in RELEASES.md?
No