-
Notifications
You must be signed in to change notification settings - Fork 2
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
ZK-1337: Publish packages to npm #87
Conversation
|
required: true | ||
type: choice | ||
options: | ||
- develop |
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.
just making sure that develop
is an actual tag and not a misspelling of development
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.
an actual tag
elif [ "${{ github.event.inputs.package }}" = "shielder-sdk-crypto" ]; then | ||
pnpm build-package:crypto | ||
else | ||
pnpm build-package:crypto-wasm |
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.
nit: you could handle crypto-wasm
in another elif
and stop the workflow in the else
branch, just to have slightly more bug-resistant code in case this script evolves
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.
yes, I believe the best scenario is to have separate jobs declarations for each package. but now let's keep it simple
|
||
jobs: | ||
build-and-push-relayer: | ||
name: Build and push shielder-relayer |
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.
just making sure that the removal of a script to push relayer is deliberate
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.
yes, we don't need to push relayer this way. it was supposed only for image push to docker registry so devs can download it, not for deployments.
we now use staging envs, so we don't need this functionality
example workflows (actually pushed to npm):