-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore(ui): TE-2388 added build check and test run #1535
Conversation
with: | ||
sparse-checkout: | | ||
thirdeye-ui | ||
pom.xml |
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.
curious on why you need pom.xml and .mvn ?
I remember some time ago we made the java build tools and the frontend tools totally separated.
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 was referencing the publish action and in that its used for distribution step. And yes we don't need it in this action. Will remove it.
on: | ||
push: | ||
branches: | ||
- '!master' |
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.
do we have tests running in another CI in master as of today (can't see it here:
npm ci |
if not I think we should also run in master - we will have tests for the edge cases of:
- user forcing a PR to be merged before PR tests pass
- user pushing to master directly
the CI should be cheap so no need to over optimize here, I'd rather be on the safe side.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
cache: "npm" | ||
cache-dependency-path: './thirdeye-ui/package-lock.json' | ||
|
||
- name: Install dependencies, run test, and 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.
Wouldn't it be better to do this in separate steps? Can you please check if, when you create separate steps, the installed node_modules
gets cached in the GitHub action's artifacts?
Issue(s)
https://startree.atlassian.net/browse/TE-2388
Description
Added build check and test run for pushed to PRs other then master