feat: Move from circleci to github actions #504

uxkjaer · 2022-10-06T13:28:01Z

Todo:

Update readme to allow people to use act
remove circleci dependency
fix node18 flow with the openssl flag by removing it after ci
reenable integration tests to run cross platform (runs-on: [ubuntu-latest, windows-latest, macos-latest])

uxkjaer · 2022-10-18T12:45:24Z

fixes #342, #279

petermuessig

Small improvements needed, but overall looks GTM!

petermuessig · 2022-10-20T10:43:34Z

.github/workflows/pr.yml

+on:
+
+  pull_request:
+    branches: [ "master" ]


After this change is merged we could also go ahead and rename master to main!

petermuessig · 2022-10-20T11:12:21Z

.github/workflows/release.yml

+
+on:
+
+  push:


https://github.com/SAP/generator-easy-ui5/blob/main/.github/workflows/release.yml#L5-L8

petermuessig · 2022-10-20T11:14:44Z

.github/workflows/release.yml

+        sudo apt-get install libxss1
+        yarn --pure-lockfile
+        yarn run ci
+        echo "//registry.npmjs.org/:_authToken=$NPM_TOKEN" >> ~/.npmrc


TODO: @petermuessig to provide the NPM Token of a UI5 bot user

petermuessig · 2022-10-20T11:18:50Z

.github/workflows/release.yml

+      with:
+        path: ./packages/vscode-ui5-language-assistant
+    - name: "Publish Release on GitHub"
+      uses: "marvinpinto/action-automatic-releases@latest"


maybe we can freeze the version for the action instead of using latest

bd82 · 2022-10-30T11:48:58Z

yarn.lock

@@ -8841,11 +8841,6 @@ tr46@^1.0.1:
  dependencies:
    punycode "^2.1.0"



why did yarn.lock change if no changes i package.json?

bd82 · 2022-10-30T11:50:24Z

packages/vscode-ui5-language-assistant/test/run-integration-tests.ts

-      //   extensionDevelopmentPath,
-      //   extensionTestsPath: path,
-      // });
+      // console.warn(


please delete no longer relevant code, do not keep it in "commented out" mode...

bd82 · 2022-10-30T11:51:34Z

CONTRIBUTING.md

 ### Release Process

 Performing a release requires push permissions to the repository.

 - Ensure you are on `master` branch and synced with origin.
 - `yarn run release:version`
 - Follow the lerna CLI instructions.
- Track the new `/v\d+.\d+.\d+/` tag build on circle-ci.


should not this step be replaced (instead of removed) with tracking the build of the relevant github action?

bd82 · 2022-10-30T11:54:48Z

.github/workflows/pr.yml

+    strategy:
+      matrix:
+        node-version: [14.x, 16.x, 18.x]
+        os: [macos-latest, ubuntu-latest, windows-latest]


I think this matrix is too large considering the free tier of github actions has
strict limits on max parallel jobs and this limits are counted towards the whole SAP org afaik

https://docs.github.com/en/actions/learn-github-actions/usage-limits-billing-and-administration

bd82 · 2022-10-30T11:56:25Z

.github/workflows/pr.yml

+      with:
+        node-version: ${{ matrix.node-version }}
+        cache: 'yarn'
+    - name: Setup yarn


afaik setup-node action supports yarn so no need for a separate yarn install step

https://github.com/actions/setup-node

bd82 · 2022-10-30T11:57:54Z

.github/workflows/pr.yml

+      run: |
+            npm install -g yarn
+            yarn
+    - name: Build Node v18 - Linux


not sure if running on linux/mac/windows provides enough value to be worth the added complexity here...

bd82 · 2022-10-30T11:59:55Z

.github/workflows/pr.yml

+      if: ${{ (matrix.node-version != '18.x') && (runner.os != 'Linux') }}
+      run: yarn run ci
+
+  compliance:


does not this mean the compliance step would run for each "element" of the build matrix? This seems like a waste, I would recommand to only run it once in a separate github actions flow.

bd82 · 2022-10-30T12:00:49Z

.github/workflows/pr.yml

@@ -0,0 +1,61 @@
+name: UI5-Language-Assistant


should not the name describe the current flow?

bd82 · 2022-10-30T12:00:58Z

.github/workflows/release.yml

@@ -0,0 +1,80 @@
+name: UI5-Language-Assistant


should not the name describe the current flow?

bd82 · 2022-10-30T12:01:25Z

.github/workflows/release.yml

+      with:
+        node-version: 16.x
+        cache: 'yarn'
+    - name: Setup yarn


uncertain if a setup step is needed when using setup-node

bd82 · 2022-10-30T12:04:44Z

.github/workflows/release.yml

+        prerelease: false
+        title: "v${{ steps.package-version.outputs.current-version}}"
+        files: |
+          ${{steps.download.outputs.download-path}}/*


why is this step almost x2 the size of a similar publish/ghr flow?

https://github.com/SAP/app-studio-toolkit/blob/main/.github/workflows/release.yml

What advantages does this implementation provide over the simpler one?

bd82 · 2022-10-30T12:07:25Z

I suspect the actions should be simplified.

smaller matrix.
remove redundant steps
remove complexity around specific OS versions.
separate the concern of bringing back e2e tests and the transition to github actions.
...

Do you see anything you can use as a template in these workflows?

https://github.com/SAP/app-studio-toolkit/tree/main/.github/workflows

cla-assistant · 2023-10-18T14:31:23Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

cla-assistant · 2023-10-18T14:31:33Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

cla-assistant · 2023-10-18T14:31:47Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

marufrasully · 2023-12-08T14:42:32Z

@uxkjaer we can close this PR. The repo use already github action. Thanks.

uxkjaer added 12 commits October 6, 2022 23:13

feat: move from circleci to github actions

adac4cc

feat: changed prerelease to false

71708ee

feat: updating node18 unset options

db0ec6e

feat: added act documentation to contributing

ef2942d

refactor: set openssl-legacy in build for 18

489258f

refactor: changed build step names

0dece75

refactor: deleted circleci

270bb73

refactor: enable integration tests

f3ab452

refactor: bad yaml

ba1ed15

refactor: bad yaml again

5d1bd3b

refactor: end if properly

958d0d1

refactor: added more proper naming to steps

38b15cb

uxkjaer marked this pull request as ready for review October 18, 2022 12:34

uxkjaer requested a review from bd82 October 18, 2022 12:34

This was linked to issues Oct 18, 2022

Incorrect coverage configuration for the VSCode Ext package #279

Open

E2E VSCode tests are skipped/disabled #342

Open

petermuessig reviewed Oct 20, 2022

View reviewed changes

bd82 reviewed Oct 30, 2022

View reviewed changes

yarn.lock

@@ -8841,11 +8841,6 @@ tr46@^1.0.1:

dependencies:

punycode "^2.1.0"

Copy link

Member

bd82 Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did yarn.lock change if no changes i package.json?

bd82 reviewed Oct 30, 2022

View reviewed changes

.github/workflows/pr.yml

@@ -0,0 +1,61 @@

name: UI5-Language-Assistant

Copy link

Member

bd82 Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not the name describe the current flow?

bd82 reviewed Oct 30, 2022

View reviewed changes

.github/workflows/release.yml

@@ -0,0 +1,80 @@

name: UI5-Language-Assistant

Copy link

Member

bd82 Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not the name describe the current flow?

bd82 reviewed Oct 30, 2022

View reviewed changes

uxkjaer closed this Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Move from circleci to github actions #504

feat: Move from circleci to github actions #504

uxkjaer commented Oct 6, 2022 •

edited

uxkjaer commented Oct 18, 2022

petermuessig left a comment

petermuessig Oct 20, 2022

petermuessig Oct 20, 2022

petermuessig Oct 20, 2022

petermuessig Oct 20, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022 •

edited

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022 •

edited

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 Oct 30, 2022

bd82 commented Oct 30, 2022

cla-assistant bot commented Oct 18, 2023 •

edited

cla-assistant bot commented Oct 18, 2023

cla-assistant bot commented Oct 18, 2023

marufrasully commented Dec 8, 2023

		@@ -8841,11 +8841,6 @@ tr46@^1.0.1:
		dependencies:
		punycode "^2.1.0"


		on:

		push:

feat: Move from circleci to github actions #504

feat: Move from circleci to github actions #504

Conversation

uxkjaer commented Oct 6, 2022 • edited

uxkjaer commented Oct 18, 2022

petermuessig left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bd82 Oct 30, 2022 • edited

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

bd82 Oct 30, 2022 • edited

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

bd82 commented Oct 30, 2022

cla-assistant bot commented Oct 18, 2023 • edited

cla-assistant bot commented Oct 18, 2023

cla-assistant bot commented Oct 18, 2023

marufrasully commented Dec 8, 2023

uxkjaer commented Oct 6, 2022 •

edited

bd82 Oct 30, 2022 •

edited

bd82 Oct 30, 2022 •

edited

cla-assistant bot commented Oct 18, 2023 •

edited