Chore: Add Circle CI #32

oscar60310 · 2022-08-16T10:18:10Z

Add Circle CI config.

Add workflows for Linter and Unit test.
Fix some lint issues.
Disable some rules:
- @typescript-eslint/no-non-null-assertion
- @typescript-eslint/no-explicit-any
set allowCircularSelfDependency to ture
Add codecov https://app.codecov.io/gh/Canner/vulcan

…etween 20000 ~ 30000 change server random port from internal.port to number between 20000 ~ 30000 to avoid access issues.

- fix lint issues - set allowCircularSelfDependency to ture, we can use local path maps because we'll tranform them while building. - disable rule @typescript-eslint/no-explicit-any - disable rule @typescript-eslint/no-non-null-assertion

kokokuo

Beside some suggestion and questions, others LTGM 👍 Thanks for adding the Circle CI 😄

kokokuo · 2022-08-18T05:18:40Z

.circleci/config.yml

@@ -0,0 +1,63 @@
+version: 2.1
+
+defaults: &defaults


Great for using YAML anchor & alias to be like variable 👍

But you could use circleci built-in executors to define container for writeing it to make it readable than default and YAML anchors and alias syntax, also executors could provide multiple executor if we need in the furture.

executors: node-executor: working_directory: ~/repo docker: - image: cimg/node:16.15.1 install: executor: node-executor steps: - checkout lint: executor: node-executor steps: - checkout

It looks great to use executors! I'll update them.

kokokuo · 2022-08-18T05:19:56Z

.circleci/config.yml

+  docker:
+    - image: cimg/node:16.15.1
+
+set_env: &set_env


Suggesting to add a simple comment to hint you will use the set_env variable which defined some steps options in below steps of jobs.

kokokuo · 2022-08-18T05:20:46Z

.circleci/config.yml

+    source $BASH_ENV
+    echo $AFFECTED_ARGS
+
+yarn_cache: &yarn_cache


Same as above set_env suggestion

kokokuo · 2022-08-18T05:20:50Z

.circleci/config.yml

+    # fallback to using the latest cache if no exact match is found
+    - node-deps-node16-
+
+yarn_install: &yarn_install


Same as above set_env suggestion

kokokuo · 2022-08-18T05:29:13Z

.circleci/config.yml

@@ -31,6 +31,19 @@ yarn_install: &yarn_install
  name: Install Dependencies
  command: yarn install --frozen-lockfile --non-interactive

+codecov_install: &codecov_install


Great for adding the codecov for showing code coverage after each PR changed and tested 👍

Maybe you could rename the Install codecov to Upload the code coverage data to codecov to prevent confusion that you show the install codecov, but the command is to upload the code coverage.

NIT: If possible, maybe we could use circleci codecov orb which like GitHub Action, for making it simple

Great for adding the codecov for showing code coverage after each PR changed and tested 👍

Maybe you could rename the Install codecov to Upload the code coverage data to codecov to prevent confusion that you show the install codecov, but the command is to upload the code coverage.

The script here downloads the binary file from codecov and check the signature, It doesn't upload any data~ We upload data in the last step of test job: - run: ./codecov -t ${CODECOV_TOKEN}, I'll add some comment here.

NIT: If possible, maybe we could use circleci codecov orb which like GitHub Action, for making it simple

To use 3rd party orb we need to update our organization security policy, it might affect other projects of Canner, so I download the file manually.

https://app.circleci.com/pipelines/github/Canner/vulcan/53/workflows/eb92a347-0a49-4e3e-bb56-a097d77bbd53/jobs/76

Ohoh, got it!
It my misunderstanding, thanks for the explanation, also thanks for telling me the reason why not use the 3rd party and provide tested link too 😺

kokokuo · 2022-08-18T05:35:12Z

.circleci/config.yml

+    echo 'Fetching Base Commit from GitHub'
+    echo 'export CIRCLE_PR_NUMBER="${CIRCLE_PR_NUMBER:-${CIRCLE_PULL_REQUEST##*/}}"' >> $BASH_ENV
+    source $BASH_ENV
+    echo "export CIRCLE_PR_BASE_SHA=`curl -s https://api.github.com/repos/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/pulls/${CIRCLE_PR_NUMBER} | jq -r '.base.sha'`" >> $BASH_ENV
+    echo 'export AFFECTED_ARGS="--base=${CIRCLE_PR_BASE_SHA}"' >> $BASH_ENV
+    echo 'export NODE_OPTIONS="--max_old_space_size=6144"' >> $BASH_ENV
+
+    source $BASH_ENV
+    echo $AFFECTED_ARGS


Would you like to get the current PR base to commit SHA and keep "--base=${CIRCLE_PR_BASE_SHA}" to AFFECTED_ARGS ( I guess you refer the nx-example code ) to let the nx affected could lint the current PR changed the part and test the current PR changed part ( in the next commit )?

Btw, maybe you could add a comment to hint to us what the part you would like to do for what purpose e.g: get the current PR commit SHA for nx affected could lint and test the only current PR changed code.

Would you like to get the current PR base to commit SHA and keep "--base=${CIRCLE_PR_BASE_SHA}" to AFFECTED_ARGS ( I guess you refer the nx-example code ) to let the nx affected could lint the current PR changed the part and test the current PR changed part ( in the next commit )?

I don't quite get what you mean 🤣 , did you mean what's the reason of saving base SHA? Yes, I want to let CI only test/lint the changed packages, i.e. the difference from HEAD to base.

Btw, maybe you could add a comment to hint to us what the part you would like to do for what purpose e.g: get the current PR commit SHA for nx affected could lint and test the only current PR changed code.

Added.

Yes, it my question 😆, thanks for still explaining to me 👍

kokokuo · 2022-08-18T05:38:41Z

.eslintrc.json

@@ -16,15 +16,19 @@
                "sourceTag": "*",
                "onlyDependOnLibsWithTags": ["*"]
              }
-            ]
+            ],
+            "allowCircularSelfDependency": true


Just curious, why did we add the allowCircularSelfDependency, have our code happen the circular self-dependency, or just to add for preventing it happened in the future, or other reason ?

NX linter blocks the relative imports from ourself if this value = false, e.g. "@vulcan-sql/core/model" from core package.
https://github.com/nrwl/nx/blob/master/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts#L184

Oh, I see, thanks for sharing the source code 🤣 and replying my question 👍

oscar60310 · 2022-08-18T07:25:08Z

HI @kokokuo , all issues have been fixed, thanks!

kokokuo · 2022-08-18T07:46:38Z

Thanks for @oscar60310 replying my question!

oscar60310 force-pushed the chore/init-ci branch 10 times, most recently from aeb5f80 to c7f0e75 Compare August 17, 2022 06:25

oscar60310 changed the title ~~[WIP] Chore: Add circle ci~~ Chore: Add Circle CI Aug 17, 2022

oscar60310 requested a review from kokokuo August 17, 2022 06:30

oscar60310 marked this pull request as ready for review August 17, 2022 06:30

oscar60310 force-pushed the chore/init-ci branch from c7f0e75 to 8f331ac Compare August 17, 2022 06:43

oscar60310 force-pushed the feature/cli branch from 2fa3db9 to ab072be Compare August 17, 2022 06:50

oscar60310 force-pushed the chore/init-ci branch 2 times, most recently from 42b6663 to 1c078ef Compare August 17, 2022 10:32

Canner deleted a comment from codecov-commenter Aug 17, 2022

oscar60310 marked this pull request as draft August 17, 2022 10:33

oscar60310 changed the title ~~Chore: Add Circle CI~~ [WIP] Chore: Add Circle CI Aug 17, 2022

oscar60310 removed the request for review from kokokuo August 17, 2022 10:33

oscar60310 force-pushed the chore/init-ci branch from 1c078ef to 665286d Compare August 17, 2022 10:37

oscar60310 changed the title ~~[WIP] Chore: Add Circle CI~~ Chore: Add Circle CI Aug 18, 2022

oscar60310 force-pushed the chore/init-ci branch from 665286d to ed9a2d1 Compare August 18, 2022 01:21

oscar60310 changed the base branch from feature/cli to develop August 18, 2022 01:22

oscar60310 marked this pull request as ready for review August 18, 2022 01:28

oscar60310 requested a review from kokokuo August 18, 2022 01:29

oscar60310 added 3 commits August 18, 2022 10:37

test(serve): change server random port from internal.port to number b…

b335c79

…etween 20000 ~ 30000 change server random port from internal.port to number between 20000 ~ 30000 to avoid access issues.

fix: fix lint issues and disable some rules

16614da

- fix lint issues - set allowCircularSelfDependency to ture, we can use local path maps because we'll tranform them while building. - disable rule @typescript-eslint/no-explicit-any - disable rule @typescript-eslint/no-non-null-assertion

ci: add linter for pr-check workflows

1de84d9

oscar60310 added 2 commits August 18, 2022 10:39

ci: add test for pr-check workflows

f8611ae

ci: add codecov

f34c021

oscar60310 force-pushed the chore/init-ci branch from ed9a2d1 to f34c021 Compare August 18, 2022 02:40

fix(core): fix unused imports

c148f88

kokokuo approved these changes Aug 18, 2022

View reviewed changes

ci: update circle config and add more descriptions about CI scripts

4dcfc8f

kokokuo merged commit 7404eac into develop Aug 18, 2022

kokokuo deleted the chore/init-ci branch August 18, 2022 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: Add Circle CI #32

Chore: Add Circle CI #32

oscar60310 commented Aug 16, 2022 •

edited

Loading

kokokuo left a comment

kokokuo Aug 18, 2022 •

edited

Loading

oscar60310 Aug 18, 2022

kokokuo Aug 18, 2022

kokokuo Aug 18, 2022

kokokuo Aug 18, 2022

kokokuo Aug 18, 2022 •

edited

Loading

oscar60310 Aug 18, 2022

kokokuo Aug 18, 2022

kokokuo Aug 18, 2022

oscar60310 Aug 18, 2022

kokokuo Aug 18, 2022 •

edited

Loading

kokokuo Aug 18, 2022

oscar60310 Aug 18, 2022 •

edited

Loading

kokokuo Aug 18, 2022

oscar60310 commented Aug 18, 2022

kokokuo commented Aug 18, 2022

Chore: Add Circle CI #32

Chore: Add Circle CI #32

Conversation

oscar60310 commented Aug 16, 2022 • edited Loading

kokokuo left a comment

Choose a reason for hiding this comment

kokokuo Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 commented Aug 18, 2022

kokokuo commented Aug 18, 2022

oscar60310 commented Aug 16, 2022 •

edited

Loading

kokokuo Aug 18, 2022 •

edited

Loading

kokokuo Aug 18, 2022 •

edited

Loading

kokokuo Aug 18, 2022 •

edited

Loading

oscar60310 Aug 18, 2022 •

edited

Loading