ci: Migrate from CircleCI to GitHub Actions#7754
ci: Migrate from CircleCI to GitHub Actions#7754camdecoster wants to merge 20 commits intomasterfrom
Conversation
.circleci/download_google_fonts.py
Outdated
There was a problem hiding this comment.
@camdecoster It might be a good idea to keep this script, as a record of the source of these font files.
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - uses: browser-actions/setup-chrome@v2 |
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - uses: actions/setup-python@v6 |
|
@camdecoster Something to consider (as discussed): Using separate baseline images for the |
| const handler = (d) => { | ||
| Lib.clearThrottle(); | ||
| const isNode = d.points[0].hasOwnProperty('sourceLinks'); | ||
| const isExpectedType = (elType === 'node') ? isNode : !isNode; | ||
| if (!isExpectedType) { | ||
| gd.once(eventType, handler); | ||
| return; | ||
| } | ||
| resolve(d); | ||
| }); | ||
| } | ||
| gd.once(eventType, handler); |
There was a problem hiding this comment.
@camdecoster Can you explain this change? I can't quite get my head around it.
| expect(newPosition.x).toBeCloseTo(pos[0], -1, 'x position ' + msg); | ||
| expect(newPosition.y).toBeCloseTo(pos[1], -1, 'y position ' + msg); |
There was a problem hiding this comment.
I'm a little surprised this change is needed, since I would expect the initial position to match even if the position after drag is a little different. How many pixels off is it?
| [0, 191, 'Personnel expenses'], | ||
| [0, 179, 'Other expenses'], |
There was a problem hiding this comment.
A little surprised by this one as well... two additional points selected seems like a significant change, more than just a slight pixel difference. I'm curious what the cause is. I realize it might not be a good use of time to dig too deeply into this one though.
| @@ -2705,13 +2707,14 @@ describe('Test select box and lasso per trace:', function() { | |||
| assertPoints([ | |||
| [0, 331.5, 'Author: etpinard'], | |||
| [1, 53.5, 'Pull requests'], | |||
| [1, 15.5, 'Author: etpinard'], | |||
There was a problem hiding this comment.
Likewise, the points are spaced far enough apart on this one that I'm surprised by the change.
| - name: Build dist/ | ||
| run: npm run build | ||
|
|
||
| - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 |
There was a problem hiding this comment.
Looks like this gives us a link to download an archive of the entire dist/ folder, but not a direct link to the plotly.js bundle (the type of link we'd need to use in a codepen for example).
Could you look into how complicated it is to upload an artifact and link to it directly? I realize GHA has different limitations on artifacts than CircleCI. But I think we do need some kind of solution for linking directly to the plotly.js file generated by this job, since we use that so heavily in our dev process.
| - name: Pack tarball | ||
| run: | | ||
| npm pack | ||
| version=$(node --eval "console.log(require('./package.json').version)") | ||
| mv plotly.js-$version.tgz plotly.js.tgz | ||
|
|
||
| - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 | ||
| with: | ||
| name: tarball-node18 | ||
| retention-days: 7 | ||
| path: plotly.js.tgz |
There was a problem hiding this comment.
I realize these steps are taken directly from the CircleCI workflow but I'm not sure if they're actually needed. Maybe we can remove?
| path: stackgl_modules/index.js | ||
|
|
||
| test-topojson-build: | ||
| if: github.event_name == 'push' && github.ref_name == github.event.repository.default_branch |
There was a problem hiding this comment.
Does this mean "run only if triggered by push to default branch" ?
2 participants
Description
Migrate automated testing from CircleCI to GitHub Actions.
Closes #7732.
Changes
Testing
Notes