Skip to content
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

feat: Move from circleci to github actions #504

Closed
wants to merge 12 commits into from
135 changes: 0 additions & 135 deletions .circleci/config.yml

This file was deleted.

61 changes: 61 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: UI5-Language-Assistant
Copy link
Member

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?


on:

pull_request:
branches: [ "master" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

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


jobs:
build:
env:
HUSKY_SKIP: "true"
# NODE_OPTIONS: "--openssl-legacy-provider"
strategy:
matrix:
node-version: [14.x, 16.x, 18.x]
os: [macos-latest, ubuntu-latest, windows-latest]
Copy link
Member

Choose a reason for hiding this comment

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

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


runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- name: Setup yarn
Copy link
Member

Choose a reason for hiding this comment

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

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

run: |
npm install -g yarn
yarn
- name: Build Node v18 - Linux
Copy link
Member

Choose a reason for hiding this comment

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

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

if: ${{ (matrix.node-version == '18.x') && (runner.os == 'Linux') }}
run: xvfb-run -a yarn run ci
env:
NODE_OPTIONS: "--openssl-legacy-provider"
- name: Build Node v18 - Others OS
if: ${{ (matrix.node-version == '18.x') && (runner.os != 'Linux') }}
run: yarn run ci
env:
NODE_OPTIONS: "--openssl-legacy-provider"
- name: Build Node v14/16 - Linux
if: ${{ (matrix.node-version != '18.x') && (runner.os == 'Linux') }}
run: xvfb-run -a yarn run ci
- name: Build Node v14/16 - Other OS
if: ${{ (matrix.node-version != '18.x') && (runner.os != 'Linux') }}
run: yarn run ci

compliance:
Copy link
Member

@bd82 bd82 Oct 30, 2022

Choose a reason for hiding this comment

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

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.

runs-on: ubuntu-latest
env:
HUSKY_SKIP: "true"
steps:
- uses: actions/checkout@v3

- name: compliance check
run: |
sudo apt-get update
sudo apt-get -y install python3-pip
pip3 install --user reuse
~/.local/bin/reuse lint

80 changes: 80 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
name: UI5-Language-Assistant
Copy link
Member

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?


on:

push:
Copy link
Contributor

Choose a reason for hiding this comment

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

branches: [ "master" ]

jobs:
deploy-npm:
runs-on: ubuntu-latest
env:
HUSKY_SKIP: "true"
# NODE_OPTIONS: "--openssl-legacy-provider"
steps:
- uses: actions/checkout@v3
- name: Use Node.js 16.x
uses: actions/setup-node@v3
with:
node-version: 16.x
cache: 'yarn'
- name: Setup yarn
Copy link
Member

Choose a reason for hiding this comment

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

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

run: npm install -g yarn
- name: deploy-npm
run: |
sudo apt-get update
sudo apt-get install libxss1
yarn --pure-lockfile
yarn run ci
echo "//registry.npmjs.org/:_authToken=$NPM_TOKEN" >> ~/.npmrc
Copy link
Contributor

Choose a reason for hiding this comment

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

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

# To help debug when lerna detects changes to the working tree and fails the publish
git status
# https://github.com/lerna/lerna/issues/2788
yarn run release:publish --no-verify-access

- name: prepare-vsix
uses: actions/upload-artifact@v3
with:
name: vsix
path: ./packages/vscode-ui5-language-assistant/vscode-ui5-language-assistant-*
if-no-files-found: error # 'warn' or 'ignore' are also available, defaults to `warn`




deploy-gh-releases:
needs: deploy-npm
runs-on: ubuntu-latest
env:
HUSKY_SKIP: "true"
# NODE_OPTIONS: "--openssl-legacy-provider"
steps:
- uses: actions/checkout@v3

- name: Use Node.js 16.x
uses: actions/setup-node@v3
with:
node-version: 16.x
cache: 'yarn'
- name: Setup yarn
run: npm install -g yarn
- uses: actions/download-artifact@v3
with:
name: vsix

- name: Inspect dist Folder
run: ls -R
- name: get-npm-version
id: package-version
uses: martinbeentjes/npm-get-version-action@main
with:
path: ./packages/vscode-ui5-language-assistant
- name: "Publish Release on GitHub"
uses: "marvinpinto/action-automatic-releases@latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

with:
repo_token: "${{ secrets.GITHUB_TOKEN }}"
automatic_release_tag: ${{ steps.package-version.outputs.current-version}}
prerelease: false
title: "v${{ steps.package-version.outputs.current-version}}"
files: |
${{steps.download.outputs.download-path}}/*
Copy link
Member

Choose a reason for hiding this comment

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

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

What advantages does this implementation provide over the simpler one?

20 changes: 18 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,31 @@ for each package and automatically generate the changelog by adhering to [Conven
[lerna-mode]: https://github.com/lerna/lerna#independent-mode
[cc]: https://www.conventionalcommits.org/en/v1.0.0/

### Prerelease testing

You can test the CI flow using [act](https://github.com/nektos/act). Follow the installation process and use command

#### Pull Requests

```
act --detect-event pull-request
```

#### Complete push to master

Prerequisite: Create a folder /tmp/ghartifacts or change the path below

```
act --detect-event push --artifact-server-path /tmp/ghartifacts
```

### 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.
Copy link
Member

Choose a reason for hiding this comment

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

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

- https://circleci.com/gh/SAP/ui5-language-assistant.
- Once the tag builds have successfully finished:
- Inspect the npm registry to see the new sub packages versions.
- Inspect the new github release named after the new `/v\d+.\d+.\d+/` tag
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ async function main(): Promise<void> {
const scenarioPaths = await globby(`${testPkgFolder}/**/index.js`);
// Use for of + await to ensure running in sequence because vscode-test library cannot start multiple vscode instances at the same time
for (const path of scenarioPaths) {
console.warn(
`SKIPPING TEST: ${path}.\nsee: https://github.com/SAP/ui5-language-assistant/issues/342`
);
// await runTests({
// extensionDevelopmentPath,
// extensionTestsPath: path,
// });
// console.warn(
Copy link
Member

@bd82 bd82 Oct 30, 2022

Choose a reason for hiding this comment

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

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

// `SKIPPING TEST: ${path}.\nsee: https://github.com/SAP/ui5-language-assistant/issues/342`
// );
await runTests({
extensionDevelopmentPath,
extensionTestsPath: path,
launchArgs: ["--disable-extensions"],
});
}
} catch (err) {
console.error("Failed to run tests: ", err);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {

const EXTENSION_START_TIMEOUT = 5000;

describe.skip("the Language Server Client Validations Integration Tests - Flex Disabled", () => {
describe("the Language Server Client Validations Integration Tests - Flex Disabled", () => {
const scenarioPath = resolve(
rootPkgFolder,
"test",
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8841,11 +8841,6 @@ tr46@^1.0.1:
dependencies:
punycode "^2.1.0"

Copy link
Member

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?

tr46@~0.0.3:
version "0.0.3"
resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a"
integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==

"traverse@>=0.3.0 <0.4":
version "0.3.9"
resolved "https://registry.yarnpkg.com/traverse/-/traverse-0.3.9.tgz#717b8f220cc0bb7b44e40514c22b2e8bbc70d8b9"
Expand Down