-
Notifications
You must be signed in to change notification settings - Fork 212
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
ci(integration): add a step to archive core eval scripts #9929
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
Where should we expect to find the artifacts? |
.github/workflows/integration.yml
Outdated
with: | ||
name: core-proposals | ||
path: ./*.zip | ||
- name: remove all zip files after they're archived |
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 required to pass the git tree dirty check later on in this workflow
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.
could you create the zips in a path outside the repo? e.g. /tmp/proposal-zips
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.
or in the repo but under a .gitignored path. e.g. build
. That should avoid the dirty check.
We shouldn't need another step for cleanup
c7d997c
to
3b9b28f
Compare
I'm planning to add following comments to the yaml file, is the explanation clear enough for you? An successful run from previous commit can be seen here: https://github.com/Agoric/agoric-sdk/actions/runs/10494325003
|
.github/workflows/integration.yml
Outdated
uses: actions/upload-artifact@v4 | ||
with: | ||
name: core-proposals | ||
path: ./*.zip |
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.
why zip instead of uploading the dirs?
path: ./a3p-integration/proposals/*/submission
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.
Because upload-artifact
action doesn't like it when there's :
in the path, we have b:upgrade-next
in the path which would prevent the action from uploading. So I opted to zip while replacing the :
with -
instead of renaming the directories
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. Please document that in a comment on the zipping step
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.
Alternately, it would be less code and perhaps a better debugging experience if instead of zipping the submissions were copied to a /tmp folder and this step uploaded from there. (copy with rename of parent path)
- collect core proposals after proposals tests - collect core proposals under /tmp/core_proposals - add comments on how to find the archived core proposals
Thanks to suggestions by @turadg, the file structure for the artifact is cleaner too!
|
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 working to my satisfaction.
Looks like @turadg has some helpful suggestions on comments etc; for now, I'll stand by for that discussion to conclude before approving.
as that is more accurate
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.
LGTM! Be sure to automerge:squash
so keep the history clean in master
ad-hoc PR ## Description We'd like to archive core proposals as they're built in test-docker-build job ### Security Considerations No secrets should be archived by this step ### Scaling Considerations ### Documentation Considerations ### Testing Considerations Will run integration tests in this PR to test this change ### Upgrade Considerations
ad-hoc PR
Description
We'd like to archive core proposals as they're built in test-docker-build job
Security Considerations
No secrets should be archived by this step
Scaling Considerations
Documentation Considerations
Testing Considerations
Will run integration tests in this PR to test this change
Upgrade Considerations