-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
format: airbyte-ci format fix all
on PR / airbyte-ci format check all
on master
#32491
Changes from all commits
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,59 @@ | ||
name: Check for formatting errors on head ref | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref }} | ||
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
branches: | ||
- master | ||
jobs: | ||
format-check: | ||
runs-on: "conn-prod-xlarge-runner" | ||
name: "Check for formatting errors on ${{ github.head_ref }}" | ||
timeout-minutes: 40 | ||
steps: | ||
- name: Checkout Airbyte | ||
uses: actions/checkout@v3 | ||
with: | ||
ref: ${{ github.head_ref }} | ||
token: ${{ secrets.GH_PAT_APPROVINGTON_OCTAVIA }} | ||
|
||
- name: Run airbyte-ci format check | ||
id: airbyte_ci_format_check_all | ||
uses: ./.github/actions/run-dagger-pipeline | ||
continue-on-error: true | ||
with: | ||
context: "master" | ||
docker_hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }} | ||
docker_hub_username: ${{ secrets.DOCKER_HUB_USERNAME }} | ||
gcs_credentials: ${{ secrets.METADATA_SERVICE_PROD_GCS_CREDENTIALS }} | ||
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }} | ||
github_token: ${{ secrets.GH_PAT_MAINTENANCE_OCTAVIA }} | ||
subcommand: "format check all" | ||
|
||
# This is helpful in the case that we change a previously committed generated file to be ignored by git. | ||
- name: Remove any files that have been gitignored | ||
run: git ls-files -i -c --exclude-from=.gitignore | xargs -r git rm --cached | ||
|
||
- name: Match GitHub User to Slack User | ||
id: match-github-to-slack-user | ||
uses: ./.github/actions/match-github-to-slack-user | ||
env: | ||
AIRBYTE_TEAM_BOT_SLACK_TOKEN: ${{ secrets.SLACK_AIRBYTE_TEAM_READ_USERS }} | ||
GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
- name: Format Failure on Master Slack Channel | ||
if: steps.airbyte_ci_format_check_all.outcome == 'failure' | ||
uses: abinoda/slack-action@master | ||
env: | ||
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN_AIRBYTE_TEAM }} | ||
with: | ||
args: >- | ||
{\"channel\":\"C03BEADRPNY\", \"blocks\":[ | ||
{\"type\":\"divider\"}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"Formatting is broken on master! :bangbang: \n\n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"_merged by_: *${{ github.actor }}* \n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"<@${{ steps.match-github-to-slack-user.outputs.slack_user_ids }}> \n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\" :octavia-shocked: <https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}|View Action Run> :octavia-shocked: \n\"}}, | ||
{\"type\":\"divider\"}]} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
name: Check for formatting errors | ||
name: Automatic Formatting on PRs | ||
|
||
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. You're going to want to follow the "fix" with a "check". Not everything can always be fixed automatically and formatting errors should be caught before the PR is merged. In fact, that's the DoD for this whole initiative. 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. Sure 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. Done in 715a8e0 |
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.ref }} | ||
|
@@ -7,9 +7,6 @@ concurrency: | |
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
branches: | ||
- master | ||
pull_request: | ||
jobs: | ||
format-fix: | ||
|
@@ -44,7 +41,7 @@ jobs: | |
- '**/*' | ||
- '!**/*.md' | ||
|
||
- name: Run airbyte-ci format fix | ||
- name: Run airbyte-ci format fix all | ||
uses: ./.github/actions/run-dagger-pipeline | ||
with: | ||
context: "pull_request" | ||
|
@@ -68,31 +65,13 @@ jobs: | |
commit_user_name: Octavia Squidington III | ||
commit_user_email: octavia-squidington-iii@users.noreply.github.com | ||
|
||
notify-failure-slack-channel: | ||
name: "Notify Slack Channel on Build Failures" | ||
runs-on: ubuntu-latest | ||
needs: | ||
- format-fix | ||
if: ${{ failure() && github.ref == 'refs/heads/master' }} | ||
steps: | ||
- name: Checkout Airbyte | ||
uses: actions/checkout@v3 | ||
- name: Match GitHub User to Slack User | ||
id: match-github-to-slack-user | ||
uses: ./.github/actions/match-github-to-slack-user | ||
env: | ||
AIRBYTE_TEAM_BOT_SLACK_TOKEN: ${{ secrets.SLACK_AIRBYTE_TEAM_READ_USERS }} | ||
GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
- name: Format Failure on Master Slack Channel | ||
uses: abinoda/slack-action@master | ||
env: | ||
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN_AIRBYTE_TEAM }} | ||
- name: Run airbyte-ci format check all | ||
uses: ./.github/actions/run-dagger-pipeline | ||
with: | ||
args: >- | ||
{\"channel\":\"C03BEADRPNY\", \"blocks\":[ | ||
{\"type\":\"divider\"}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"Formatting is broken on master! :bangbang: \n\n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"_merged by_: *${{ github.actor }}* \n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"<@${{ steps.match-github-to-slack-user.outputs.slack_user_ids }}> \n\"}}, | ||
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\" :octavia-shocked: <https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}|View Action Run> :octavia-shocked: \n\"}}, | ||
{\"type\":\"divider\"}]} | ||
context: "pull_request" | ||
docker_hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }} | ||
docker_hub_username: ${{ secrets.DOCKER_HUB_USERNAME }} | ||
gcs_credentials: ${{ secrets.METADATA_SERVICE_PROD_GCS_CREDENTIALS }} | ||
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }} | ||
github_token: ${{ secrets.GH_PAT_MAINTENANCE_OCTAVIA }} | ||
subcommand: "format check all" |
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.
Not a blocking comment but in the near future we're going to want to subsume a larger class of checks than just formatting checks in this workflow, so its name should reflect that.
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.
I'd prefer keeping this name as it is as we don't know yet what's going to be in this "larger class of checks" and we might want to split them in multiple workflows
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.
Good point