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

chore: migrate build from azure-pipelines to actions #1383

Merged
merged 16 commits into from
Mar 16, 2020
Merged

chore: migrate build from azure-pipelines to actions #1383

merged 16 commits into from
Mar 16, 2020

Conversation

armano2
Copy link
Member

@armano2 armano2 commented Dec 26, 2019

This is POC of build migration from azure-pipelines to actions GitHub Workflow Status


From my research about github actions:

Examples

External project test

@armano2 armano2 added DO NOT MERGE PRs which should not be merged yet RFC labels Dec 26, 2019
@typescript-eslint

This comment has been minimized.

@armano2 armano2 requested a review from a team December 26, 2019 07:00
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

@JamesHenry was the one that set up all of the azure stuff. Would like his eyeball.

Also do you know how easy it is to setup actions for things like locking old, closed issues? I get annoyed when people necro old things, but have been too lazy to investigate a bot 😅

Also we have the typescript-eslint bot which posts on PRs whenever they are opened - would that be easy to migrate?

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@armano2
Copy link
Member Author

armano2 commented Dec 26, 2019

Also do you know how easy it is to setup actions for things like locking old, closed issues? I get annoyed when people necro old things, but have been too lazy to investigate a bot 😅

yea i could set this up to, but this can be done in separate PR

about our bot, there few actions that are already prepeared for this for example:
https://github.com/marketplace/actions/first-interaction or https://github.com/marketplace/actions/close-stale-issues

name: Greetings

on: [pull_request, issues]

jobs:
  greeting:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/first-interaction@v1
      with:
        repo-token: ${{ secrets.GITHUB_TOKEN }}
        issue-message: 'Message that will be displayed on users'' first issue'
        pr-message: 'Message that will be displayed on users'' first pr'

@JamesHenry
Copy link
Member

Err...why? 😅 also we have a slack channel precisely to discuss things like this

@armano2
Copy link
Member Author

armano2 commented Dec 30, 2019

@JamesHenry i added small description

@bradzacher
Copy link
Member

bradzacher commented Dec 31, 2019

one good reason to move to github actions is because it centralises the admin, which makes it a bit more transparent.
Otherwise I see it as "the same as azure pipelines".

Same for the bots, having them as actions would be a win for transparent and easy maintaining (eg I don't know where or how the PR message bot is implemented atm).

@armano2 armano2 marked this pull request as ready for review January 2, 2020 02:53
@armano2 armano2 self-assigned this Jan 12, 2020
@codecov
Copy link

codecov bot commented Jan 28, 2020

Codecov Report

Merging #1383 into master will increase coverage by 0.3%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #1383     +/-   ##
=========================================
+ Coverage   95.19%   95.49%   +0.3%     
=========================================
  Files         148      150      +2     
  Lines        6967     6639    -328     
  Branches     2007     1880    -127     
=========================================
- Hits         6632     6340    -292     
+ Misses        124      112     -12     
+ Partials      211      187     -24
Impacted Files Coverage Δ
...pt-estree/src/ts-estree/estree-to-ts-node-types.ts 0% <0%> (-100%) ⬇️
...s/eslint-plugin/src/rules/interface-name-prefix.ts 83.33% <0%> (-16.67%) ⬇️
...-plugin/src/rules/no-unnecessary-type-assertion.ts 90.12% <0%> (-6.85%) ⬇️
packages/eslint-plugin/src/rules/unbound-method.ts 94.44% <0%> (-2.23%) ⬇️
...ackages/eslint-plugin/src/rules/no-implied-eval.ts 95.91% <0%> (-1.91%) ⬇️
packages/eslint-plugin/src/rules/typedef.ts 95.91% <0%> (-1.35%) ⬇️
...plugin/src/rules/explicit-module-boundary-types.ts 85.45% <0%> (-0.76%) ⬇️
...-plugin/src/rules/explicit-member-accessibility.ts 96.36% <0%> (-0.74%) ⬇️
...ackages/eslint-plugin/src/rules/member-ordering.ts 92.3% <0%> (-0.38%) ⬇️
...nt-plugin/src/rules/no-unused-vars-experimental.ts 91.39% <0%> (-0.1%) ⬇️
... and 41 more

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. I'm happy to merge this as a trial at the very least.
We can run both azure and github actions for a period whilst we make a greater decision

Comment on lines +12 to +58
primary_code_validation_and_tests:
name: Primary code validation and tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Use Node.js 12
uses: actions/setup-node@v1
with:
node-version: 12

# This also runs a build as part of the postinstall bootstrap
- name: install and build
run: |
yarn --ignore-engines --frozen-lockfile
yarn check-clean-workspace-after-install

# Note that this command *also* typechecks tests/tools,
# whereas the build only checks src files
- name: Typecheck all packages
run: yarn typecheck

- name: Check code formatting
run: yarn format-check

- name: Run linting
run: yarn lint

- name: Validate spelling
run: yarn check:spelling

- name: Run unit tests
run: yarn test
env:
CI: true

- name: Run integrations tests
run: yarn integration-tests
env:
CI: true

- name: Publish code coverage report
uses: codecov/codecov-action@v1
with:
yml: ./codecov.yml
token: ${{ secrets.CODECOV_TOKEN }}
flags: unittest
name: codecov
Copy link
Member

Choose a reason for hiding this comment

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

Question - did we want to split all of this up to give us more granular status checks?
I know that a bit of a pain point for people is just seeing "primary validation" failed, and then having to dig into the logs to see which step failed.

I.e. one job for each of:

  • type check
  • format check
  • lint
  • spelling
  • test + integration (+ coverage publish) test on latest node

Copy link
Member Author

@armano2 armano2 Feb 12, 2020

Choose a reason for hiding this comment

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

you actually don't have to as actions are showing it in overview screen

example: https://github.com/armano2/test-eslint-monorpo/actions/runs/36409848

you can click on Check failure on line 1 in packages/a/src/index.js if file was modified it's going to show annotation within this file

@armano2
Copy link
Member Author

armano2 commented Feb 12, 2020

as for codecov/codecov-action we should wait for codecov/codecov-action#53 to get merged first as it has same issue with PR ids as pipelines

for now we could just disable it or use bash uploader

@JamesHenry JamesHenry merged commit c969884 into typescript-eslint:master Mar 16, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
@armano2 armano2 deleted the github-actions-test branch January 12, 2021 20:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DO NOT MERGE PRs which should not be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants