-
Notifications
You must be signed in to change notification settings - Fork 31
build: Add CI configuration. Rename 'node' to 'server-node'. #55
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
Changes from all commits
d7a8c51
968738d
aad1146
1957cb4
01890a7
4a50c89
18cf3fe
117f3ab
bd1c899
43d1abe
69b316b
7acb892
16d1add
cf4cc0a
911cf04
8b87581
c4ebef1
40cbc57
e720b1d
27349c1
ad2fae2
1bb37a1
b808216
e215c52
3fcb23b
535d480
592fd52
4c821ff
25932e6
bb1ff66
6689962
2ec0ab2
7615baf
9fd2cee
0580713
56b132f
92650f3
2f49fe6
447a0f1
818dfd4
334485f
2776fee
03146cb
06147bb
76f15f7
7f617b6
e0bb5ff
b283c5d
6b6cf4a
b64282c
705e3fb
b7184e9
08ec806
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: shared/common | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' #Do not need to run CI for markdown changes. | ||
| pull_request: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' | ||
|
|
||
| jobs: | ||
| build-test-common: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 16 | ||
| registry-url: 'https://registry.npmjs.org' | ||
| - id: shared | ||
| name: Shared CI Steps | ||
| uses: ./actions/ci | ||
| with: | ||
| workspace_name: '@launchdarkly/js-sdk-common' | ||
| workspace_path: packages/shared/common |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| name: Lint PR title | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: | ||
| - opened | ||
| - edited | ||
| - synchronize | ||
|
|
||
| jobs: | ||
| main: | ||
| name: Verify the PR title matches conventional commit spec. | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: amannn/action-semantic-pull-request@v5 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: shared/sdk-server | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' #Do not need to run CI for markdown changes. | ||
| pull_request: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' | ||
|
|
||
| jobs: | ||
| build-test-sdk-server: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 16 | ||
| registry-url: 'https://registry.npmjs.org' | ||
| - id: shared | ||
| name: Shared CI Steps | ||
| uses: ./actions/ci | ||
| with: | ||
| workspace_name: '@launchdarkly/js-server-sdk-common' | ||
| workspace_path: packages/shared/sdk-server |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| name: sdk/server-node | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' #Do not need to run CI for markdown changes. | ||
| pull_request: | ||
| branches: [main] | ||
| paths-ignore: | ||
| - '**.md' | ||
|
|
||
| jobs: | ||
| build-test-server-node: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| matrix: | ||
| # Node versions to run on. | ||
| version: [16, 19] | ||
|
|
||
| env: | ||
| TEST_HARNESS_PARAMS: '--skip-from=./contract-tests/testharness-suppressions.txt' | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: ${{ matrix.version }} | ||
| registry-url: 'https://registry.npmjs.org' | ||
| - id: shared | ||
| name: Shared CI Steps | ||
| uses: ./actions/ci | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These actions are just local, but you can share them like you can share repositories. You would just put |
||
| with: | ||
| workspace_name: '@launchdarkly/node-server-sdk' | ||
| workspace_path: packages/sdk/server-node | ||
| - name: Contract Tests | ||
| run: yarn run contract-tests | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the simplest way to run the contract tests, but not the way with the best output. On circleci we run a job for the service parallel to the test service. That makes the output nice, but sharing networking between jobs is a big problem on github actions. The jobs are different VMs or containers depending. Maybe something we could figure out in the future. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "singleQuote": true, | ||
| "printWidth": 100 | ||
| } |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| nodeLinker: node-modules | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this makes it easier to keep things node friendly. |
||
|
|
||
| plugins: | ||
| - path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs | ||
| spec: '@yarnpkg/plugin-workspace-tools' | ||
|
|
||
| yarnPath: .yarn/releases/yarn-3.4.1.cjs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # This is a composite to allow sharing these steps into other workflows. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, this lets us re-use these steps easily in different workflows. The output is not as nice as a shared workflow, but this made it easy for node server to run a matrix, and also run contract tests afterward. With a shared workflow you cannot really deviate, you use it or you don't and you cannot run steps after. So this worked out better functionally. |
||
| # It isn't a shared workflow, because then it isn't convenient to add | ||
| # additional package specific steps. | ||
| name: Shared CI Workflow | ||
| inputs: | ||
| # Some commands work on the package name (yarn commands), other require the path (typedoc), | ||
| # so we supply both. | ||
| workspace_name: | ||
| required: true | ||
| type: string | ||
| workspace_path: | ||
| required: true | ||
| type: string | ||
|
|
||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Setup Yarn | ||
| shell: bash | ||
| run: yarn set version 3.4.1 | ||
|
|
||
| - name: Install Dependencies | ||
| shell: bash | ||
| # Install only the dependencies requires for the specific workspace. | ||
| run: yarn workspaces focus ${{ inputs.workspace_name }} | ||
|
|
||
| - name: Build | ||
| shell: bash | ||
| # This will build the package and its dependencies. | ||
| run: yarn workspaces foreach -ptR --from '${{ inputs.workspace_name }}' run build | ||
|
|
||
| - name: Lint | ||
| shell: bash | ||
| run: yarn workspace ${{ inputs.workspace_name }} lint | ||
|
|
||
| - name: Test | ||
| shell: bash | ||
| # Run just the tests for the targeted package. | ||
| run: yarn workspace ${{ inputs.workspace_name }} test | ||
|
|
||
| - name: Build Docs | ||
| shell: bash | ||
| run: ./scripts/build-doc.sh ${{ inputs.workspace_path }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| module.exports = { | ||
| transform: {'^.+\\.ts?$': 'ts-jest'}, | ||
| testMatch: ["**/__tests__/**/*test.ts?(x)"], | ||
| transform: { '^.+\\.ts?$': 'ts-jest' }, | ||
| testMatch: ['**/__tests__/**/*test.ts?(x)'], | ||
| testEnvironment: 'node', | ||
| moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], | ||
| collectCoverageFrom: [ | ||
| "packages/sdk/node/src/**/*.ts", | ||
| "packages/shared/common/src/**/*.ts", | ||
| "packages/shared/sdk-server/src/**/*.ts" | ||
| ] | ||
| 'packages/sdk/server-node/src/**/*.ts', | ||
| 'packages/shared/common/src/**/*.ts', | ||
| 'packages/shared/sdk-server/src/**/*.ts', | ||
| ], | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| "workspaces": [ | ||
| "packages/shared/common", | ||
| "packages/shared/sdk-server", | ||
| "packages/sdk/node" | ||
| "packages/sdk/server-node" | ||
| ], | ||
| "private": true, | ||
| "scripts": { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also add a yarn all command to install all workspaces and prettier as well? You can steal the prettier config from the docs repo. "all": "yarn workspaces focus -A",
"prettier": "prettier --write \"**/*.{js,ts,tsx,json,yaml,yml,md}\" --loglevel warn",
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For "all" is there a reason to use a focus all versus just running
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't know a plain |
||
|
|
@@ -13,19 +13,25 @@ | |
| "build:doc": "./scripts/build-doc.sh $1", | ||
| "lint": "npx eslint . --ext .ts", | ||
| "lint:fix": "yarn run lint -- --fix", | ||
| "test": "jest", | ||
| "test": "npx jest", | ||
| "coverage": "npm run test -- --coverage", | ||
| "contract-test-service": "npm --prefix contract-tests install && npm --prefix contract-tests start", | ||
| "contract-test-harness": "curl -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/master/downloader/run.sh \\ | VERSION=v2 PARAMS=\"-url http://localhost:8000 -debug -stop-service-at-end $TEST_HARNESS_PARAMS\" sh", | ||
| "contract-tests": "npm run contract-test-service & npm run contract-test-harness" | ||
| "contract-tests": "npm run contract-test-service & npm run contract-test-harness", | ||
| "prettier": "npx prettier --write \"**/*.{js,ts,tsx,json,yaml,yml,md}\" --loglevel warn" | ||
| }, | ||
| "devDependencies": { | ||
| "@typescript-eslint/eslint-plugin": "^5.22.0", | ||
| "@typescript-eslint/parser": "^5.22.0", | ||
| "eslint": "^8.14.0", | ||
| "eslint-config-airbnb-base": "^15.0.0", | ||
| "eslint-config-airbnb-typescript": "^17.0.0", | ||
| "eslint-config-prettier": "^8.7.0", | ||
| "eslint-plugin-import": "^2.26.0", | ||
| "typedoc": "0.23.26" | ||
| } | ||
| "eslint-plugin-prettier": "^4.2.1", | ||
| "prettier": "^2.8.4", | ||
| "typedoc": "0.23.26", | ||
| "typescript": "^4.6.3" | ||
| }, | ||
| "packageManager": "yarn@3.4.1" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Do we want 18 instead of 19 here? Odd releases don't go to active LTS and maintenance. Unless your intent is precisely this to try the most cutting edge, latest node release?
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.
Yeah, just the oldest and the newest it will claim to support at the moment. I may automate this as well. Technically 14 is supported until next month from node, but I don't want to support it and then immediately remove it.
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.
ok