Refactor: [fa-722] update gha build and deploy#55
Conversation
| on: | ||
| push: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| types: | ||
| - closed |
There was a problem hiding this comment.
Changed so that PR must be merged to kick off deployment.
| - name: Upload demo page | ||
| uses: actions/upload-pages-artifact@v3 | ||
| with: | ||
| path: demo/dist/ | ||
|
|
||
| # Deploy job | ||
| deploy: | ||
| # Add a dependency to the build job | ||
| needs: build | ||
|
|
||
| # Grant GITHUB_TOKEN the permissions required to make a Pages deployment | ||
| permissions: | ||
| pages: write # to deploy to Pages | ||
| id-token: write # to verify the deployment originates from an appropriate source | ||
|
|
||
| # Specify runner + deployment step | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| path: './demo/public' |
There was a problem hiding this comment.
Fix missing Github Pages
| # - name: Publish to NPMJS | ||
| # uses: JS-DevTools/npm-publish@v3 | ||
| # with: | ||
| # token: ${{ secrets.NPM_TOKEN }} No newline at end of file |
There was a problem hiding this comment.
I think this is ready but want to limit the amount of change if there is trouble
| # Grant GITHUB_TOKEN the permissions required to make a Pages deployment | ||
| permissions: | ||
| pages: write # to deploy to Pages | ||
| id-token: write # to verify the deployment originates from an appropriate source |
There was a problem hiding this comment.
Combined into a single job to simplify troubleshooting and moved permissions to the top
| "strict": true, | ||
| "outDir": "./lib", | ||
| "moduleResolution": "node" | ||
| "target": "es2015", |
There was a problem hiding this comment.
if you want to, I think we could bump this up to something more modern.
There was a problem hiding this comment.
i figured it was better to change as little as possible in case there was user impact. i don't want to wreck some IE9 user's day without at least a deprecation warning
There was a problem hiding this comment.
I guess that is fair, but flowplayer doesn't support IE, though we do support some very old iOS versions
There was a problem hiding this comment.
ah i see then it doesn't matter what my output target is, it will be chained to the "native" player's target 👍 we should sync those targets then
There was a problem hiding this comment.
we use es2017 there for now.
Fixes issues with the build and deployment to github pages