- Notifications
You must be signed in to change notification settings - Fork 1
CI Integration #12
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
CI Integration #12
Conversation
markdboyd left a comment
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.
Looks great! I have one very small requested change
| - name: Setup node.js 🏡 | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: '14.15.0' |
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.
If we're going to test against Node 14.15, let's add an .nvmrc to use that locally
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.
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.
Well then, touché
| @@ -0,0 +1,5 @@ | |||
| "use strict"; | |||
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.
Test files in TS? Oh my
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 don't actually need to do this, however I find the imports and type-completion to be a bit simpler.
| - name: Test 🧪 | ||
| run: | | ||
| npm test | ||
| | ||
| - name: 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 don't need to build before we test because we're testing against the TS directly?
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.
Right, this PR adds a transform to Jest that allows Jest to do the translating on the fly.
Lines 28 to 31 in 0b21151
| "jest": { | |
| "transform": { | |
| "^.+\\.tsx?$": "ts-jest" | |
| }, |
* Update for gh-pages integration * Add GH Actions script * Test jekyll-action * Let jekyll-action build * Tmp rm target_branch * Up version * Update Gemfile.lock * Update PAT, set target branc * Only deploy on merges to main * Rm comments and persist-credentials option * Add cache * Add quores * Avoid collections for simplicity * Rm build command * Cleanup * Revert to docs path rather than construct dir * Add test step, use granular steps * Add emoji * Fix import? * Fix name * TS based testing * Add TS dependency
This PR:
main) deploys the Jekyll documentation to thegh-pagesbranch_config.yaml