-
Notifications
You must be signed in to change notification settings - Fork 83
chore: removed travis yml and added git action support #753
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
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.
please address comments and also revert links to master.
.github/workflows/javascript.yml
Outdated
lint: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 |
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.
v3
crossbrowser_and_umd_unit_tests: | ||
runs-on: ubuntu-latest | ||
env: | ||
BROWSER_STACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }} |
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.
rename it to BROWSERSTACK_USERNAME
runs-on: ubuntu-latest | ||
env: | ||
BROWSER_STACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }} | ||
BROWSER_STACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }} |
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.
same here.
.github/workflows/javascript.yml
Outdated
BROWSER_STACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }} | ||
BROWSER_STACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }} | ||
steps: | ||
- uses: actions/checkout@v2 |
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.
update across all checkouts to v3.
.github/workflows/javascript.yml
Outdated
matrix: | ||
node: [ '8', '9', '10', '12', '14' ] | ||
steps: | ||
- uses: actions/checkout@v2 |
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.
update
path-to-lcov: ./packages/optimizely-sdk/coverage/lcov.info | ||
flag-name: run-${{ matrix.node }} | ||
# This is a parallel build so need this | ||
parallel: true |
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.
what does it mean parallel 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.
we are running unit test on multiple node versions and multiple posts on coveralls are required, thats why we are sending those posts in parallel.
"cover": "nyc report -r lcov", | ||
"precoveralls": "npm run cover", | ||
"coveralls": "< coverage/lcov.info coveralls", | ||
"coveralls": "nyc --reporter=lcov npm test", |
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 this removed?
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.
The previous precoveralls
and cover
is required to post coveralls from travis but it is not working on git actions so we post using 3rd party git action coverallsapp/github-action@master
and is no longer require 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.
lgtm
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
if: ${{ github.event_name == 'pull_request' }} | ||
run: | | ||
echo "SDK_BRANCH=${{ github.head_ref }}" >> $GITHUB_ENV | ||
echo "TRAVIS_BRANCH=${{ github.head_ref }}" >> $GITHUB_ENV |
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.
remove it.
Summary
NOTE:
Please update this, this and this line branch name to master before merging.
Test plan
All tests should pass