-
Notifications
You must be signed in to change notification settings - Fork 117
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
Release cloud-admin to netlify when tag #2057
Conversation
.github/workflows/cloud-admin.yml
Outdated
paths: | ||
- ".github/workflows/cloud-admin.yml" | ||
- "web-admin/**" | ||
- "web-local/**" |
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.
Any reason for web-local
to be included here?
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.
oh it should have been common
.github/workflows/cloud-admin.yml
Outdated
- name: Set up NodeJS | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: 18 |
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'm not sure if this is a problem or not, but I think the Applications team has all been on Node 16. (We should probably upgrade as mentioned here)
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.
we have node 16 in the tests too
https://github.com/rilldata/rill-developer/blob/main/.github/workflows/web-test.yml#L31-L34
will move all to 16 then
.github/workflows/cloud-admin.yml
Outdated
npm install | ||
npm run build -w web-admin | ||
|
||
- name: Deploy ${{ matrix.name }} to Netlify Staging |
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 matrix.name
here? I don't see a matrix build
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.
copy paste error 🤦
tags: | ||
- "*" | ||
branches: ["main"] | ||
paths: |
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 we need paths
here – any tag on main
can just trigger a build.
If you want to keep the paths
– we also need to add web-common
here
and release to prod