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

Replace mergify with GHAs #3346

Merged
merged 7 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/labeler.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
version: 1
labels:
- label: "automerge"
authors: ["softwaremill-ci"]
files:
- "build.sbt"
- "project/Versions.scala"
- "project/plugins.sbt"
- label: "dependency"
authors: ["softwaremill-ci"]
files:
- "build.sbt"
- "project/Versions.scala"
- "project/plugins.sbt"
56 changes: 51 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: CI
on:
pull_request:
branches: ['**']
push:
branches: ['**']
tags: [v*]
Expand All @@ -10,8 +9,12 @@ env:
SBT_JAVA_OPTS: -J-Xms4g -J-Xmx4g
jobs:
ci:
# run on external PRs, but not on internal PRs since those will be run by push to branch
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository
# run on 1) push, 2) external PRs, 3) softwaremill-ci PRs
# do not run on internal, non-steward PRs since those will be run by push to branch
if: |
github.event_name == 'push' ||
github.event.pull_request.head.repo.full_name != github.repository ||
github.event.pull_request.user.login == 'softwaremill-ci'
runs-on: ubuntu-22.04
strategy:
fail-fast: false
Expand Down Expand Up @@ -109,9 +112,14 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

# identify binary incompatibilities (check build.sbt for details)
mima:
Copy link
Member

@amorfis amorfis Nov 28, 2023

Choose a reason for hiding this comment

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

What is this job for?

Copy link
Member

Choose a reason for hiding this comment

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

Ensures that changes in the core module don't break binary compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

let's add a comment, if it's not clear :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

# run on external PRs, but not on internal PRs since those will be run by push to branch
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository
# run on 1) push, 2) external PRs, 3) softwaremill-ci PRs
# do not run on internal, non-steward PRs since those will be run by push to branch
if: |
github.event_name == 'push' ||
github.event.pull_request.head.repo.full_name != github.repository ||
github.event.pull_request.user.login == 'softwaremill-ci'
runs-on: ubuntu-22.04
steps:
- name: Checkout
Expand Down Expand Up @@ -182,3 +190,41 @@ jobs:
version: "v${{ env.VERSION }}"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}


# `automerge` label is attached iff there is exactly one file changed by steward and this file belongs to a
# whitelist specified by `labeler.yml`
label:
name: Attach automerge label
Copy link
Member

Choose a reason for hiding this comment

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

can we attach the dependency label? it's later used when creating the release notes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, some DRY was needed though.

Copy link
Member

Choose a reason for hiding this comment

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

though now the comment needs updating, as we attach two labels :)

Copy link
Member

Choose a reason for hiding this comment

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

or maybe we can just say this step attaches the labels specified in labelers.yml, but only if the PR changes one file

# only for PRs by softwaremill-ci
if: github.event.pull_request.user.login == 'softwaremill-ci'
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 2
# count number of files changed
- name: Count number of files changed
id: count-changed-files
run: |
N=$(git diff --name-only -r HEAD^1 HEAD | wc -w)
echo "changed_files_num=$N" >> $GITHUB_OUTPUT
- name: Launch labeler
# skip if more than one file changed
if: steps.count-changed-files.outputs.changed_files_num == 1
Copy link
Member

@amorfis amorfis Nov 28, 2023

Choose a reason for hiding this comment

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

Is it always the same file changing? If so, we should check also if this is the one that has changed. We don't want the job to run if it is other file changed do we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't see a scenario in which this "modified file swap" thing could happen - would you pls give an example?

Copy link
Member

Choose a reason for hiding this comment

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

Apparently I don't understand something. Doesn't it happen when I modify 1 file - and it's some other file than expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is white list of three files that we consider valid for "automerge" in file labeler.yml. So if steward modifies <> 1 file nothing happens, if it modifies exactly 1 one, then that file will be checked against that white list - if it is on it, then automerge label will attached, otherwise no label.

Copy link
Member

Choose a reason for hiding this comment

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

let's add all this in comments - somebody reading the build file will have no idea to look into labeler.yml :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

uses: srvaroa/labeler@master
env:
GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

auto-merge:
name: Auto merge
# only for PRs by softwaremill-ci
if: github.event.pull_request.user.login == 'softwaremill-ci'
needs: [ ci, mima, label ]
Copy link
Member

Choose a reason for hiding this comment

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

does a needs=label require that the label was successfully applied?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly - it requires to wait for label job to complete. The required label (automerge) will be checked anyway inside the action, the needs=label was added just to conform with job dependencies relation - first label, then automerge - and it protects from the (well nearly impossible) scenario when auto-merge job starts when the label job has not yet completed.

Copy link
Member

Choose a reason for hiding this comment

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

ah ok:) yeah I guess I'm missing some info on what's the flow and when various files are used

runs-on: ubuntu-22.04
steps:
- id: automerge
name: automerge
uses: "pascalgn/[email protected]"
env:
GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
19 changes: 19 additions & 0 deletions .github/workflows/rebase-cmd-dispatch.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# On any comment, it will look for '/rebase' in the comment body and in case of hit, it dispatches rebase cmd
# with event type 'rebase-command' which triggers 'rebase-command` WF that performs the rebase operation.
Copy link
Member

Choose a reason for hiding this comment

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

WF?

Copy link
Member

Choose a reason for hiding this comment

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

this doesn't explain the relation between rebase-cmd-dispatch and rebase-cmd, I still don't understand what it is

name: Slash Command Dispatch
on:
issue_comment:
types: [created]
Copy link
Member

Choose a reason for hiding this comment

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

can we only run this action if the text == /rebase? otherwise we'll get a lot of actions which do nothing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, there is no way to trigger workflow based on the content of the issue comment. Eventually we could add a condition for job like if: contains(github.event.comment.body, '/rebase') so that it will be fast-skipped but still the Action space will be polluted with tons of skipped actions triggered, each one triggered by a single comment.

Copy link
Member

Choose a reason for hiding this comment

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

I guess fast-skip is better than nothing, let's try and we'll se if it works fine for us

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

jobs:
slashCommandDispatch:
# to fast-skip for typical, non-rebase comments
if: contains(github.event.comment.body, '/rebase')
runs-on: ubuntu-latest
steps:
- name: Slash Command Dispatch
uses: peter-evans/slash-command-dispatch@v3
with:
token: ${{ secrets.GITHUB_TOKEN }}
commands: rebase
permission: write
issue-type: pull-request
20 changes: 20 additions & 0 deletions .github/workflows/rebase-cmd.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: rebase-command
on:
repository_dispatch:
types: [rebase-command]
Copy link
Member

Choose a reason for hiding this comment

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

how are these two wired together? somme comments on how this works and what are the roles of the two workflows would be great :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any comment triggers Slash Comment Dispatch which checks if there is /rebase in the comment body, if yes then it dispatches a rebase command with event type rebase-command which triggers rebase-command workflow that performs the rebase operation. Will add comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments added.

jobs:
rebase:
runs-on: ubuntu-latest
steps:
- uses: peter-evans/rebase@v2
id: rebase
with:
head: ${{ github.event.client_payload.pull_request.head.label }}
- name: Add reaction
if: steps.rebase.outputs.rebased-count == 1
uses: peter-evans/create-or-update-comment@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
repository: ${{ github.event.client_payload.github.payload.repository.full_name }}
comment-id: ${{ github.event.client_payload.github.payload.comment.id }}
reaction-type: hooray
107 changes: 0 additions & 107 deletions .mergify.yml

This file was deleted.

Loading