-
-
Notifications
You must be signed in to change notification settings - Fork 518
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: Adds build checks #1055
chore: Adds build checks #1055
Conversation
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.
Great job! 🎉 Left some small nit-picks to fix 😃
node-version: 14 | ||
cache: 'yarn' | ||
- name: Install dependencies | ||
run: yarn && cd TestsExample && yarn |
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 is yarn
run only here in the main dir of react-native-screens
and omitted in other builds? Are we certain that this job will run first and packages will be cached for others? It so I'd still split it to separate steps.
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.
It was there because before I was also running emulator and the app crashed without root dependencies.
I decided that it is not necessary to run it (without emulator job takes 3 minutes instead of 5 and can be ran on ubuntu).
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 removed the unnecessary yarn
Nice one! 🥳 |
Description
Adds build checks for iOS, tvOS and android.
Test code and steps to reproduce
Checks should pass in this PR.
They should fail in #1056.
Checklist