From 32f86da98d6c7f9d9ace4bbf5e5d499a6b9f16aa Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 27 Aug 2026 17:50:02 +0300 Subject: [PATCH] Replace danger-swift with a actions/github-script version (#6080) * Replace danger-swift with a actions/github-script version * Report PR check results in a comment Annotations are only visible in the Checks tab, so the results were easy to miss on the pull request itself. The workflow now keeps a single comment up to date on the Conversation tab, listing failures and warnings together, and deletes it again once there is nothing left to report. Commenting on pull requests from forks needs a token with write access, which the pull_request trigger does not provide, hence the switch to pull_request_target. The job never checks out or executes any code from the pull request, it only reads the event payload and the list of changed files. --- .github/workflows/danger.yml | 32 ---------- .github/workflows/pr-checks.yml | 109 ++++++++++++++++++++++++++++++++ AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- Dangerfile.swift | 52 --------------- 5 files changed, 111 insertions(+), 86 deletions(-) delete mode 100644 .github/workflows/danger.yml create mode 100644 .github/workflows/pr-checks.yml delete mode 100644 Dangerfile.swift diff --git a/.github/workflows/danger.yml b/.github/workflows/danger.yml deleted file mode 100644 index ca81d6cc9..000000000 --- a/.github/workflows/danger.yml +++ /dev/null @@ -1,32 +0,0 @@ -name: Danger CI - -on: - pull_request: - types: [labeled, synchronize, opened, reopened] - -permissions: {} - -jobs: - build: - name: Danger - runs-on: ubuntu-latest - timeout-minutes: 15 - permissions: - contents: read - pull-requests: write # The token is still read-only for forks. - statuses: write - - concurrency: - # Only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ format('danger-ci-{0}', github.ref) }} - cancel-in-progress: true - - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - name: Danger - uses: danger/swift@751aa740b2bd69ea1b54444a945961e64c38fc22 # 3.22.1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml new file mode 100644 index 000000000..8c2856b02 --- /dev/null +++ b/.github/workflows/pr-checks.yml @@ -0,0 +1,109 @@ +name: PR Checks + +on: + # Privilege escalation necessary to comment on pull requests from forks. + # 🚨 We must not check out or execute any code here, and be careful around use of user-controlled inputs. + pull_request_target: # zizmor: ignore[dangerous-triggers] + types: [opened, reopened, edited, synchronize, labeled, unlabeled] + +permissions: {} + +jobs: + pr-checks: + name: PR Checks + runs-on: ubuntu-latest + timeout-minutes: 5 + + concurrency: + # Only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ format('pr-checks-{0}', github.ref) }} + cancel-in-progress: true + + permissions: + pull-requests: write + + steps: + - name: Check the pull request + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const pr = context.payload.pull_request; + const body = pr.body ?? ""; + + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number + }); + const paths = files.map(file => file.filename.toLowerCase()); + + const warnings = []; + const failures = []; + + if (pr.additions > 1000) { + warnings.push("This pull request seems relatively large. Please consider splitting it into multiple smaller ones."); + } + + if (!body) { + warnings.push("Please provide a description for this PR."); + } + + if (paths.some(path => path.includes("/view")) && !body.includes("user-attachments")) { + warnings.push("You seem to have made changes to views. Please consider adding screenshots."); + } + + if (paths.some(path => path.includes(".xcassets") && path.endsWith(".png"))) { + warnings.push("You seem to have made changes to some resource images. Please consider using an SVG or PDF."); + } + + if (pr.title.endsWith("…") || /^(Fixes|Fix) #\d+/.test(pr.title) || /^\w+\(\w+\):/.test(pr.title)) { + failures.push("Please provide a complete title that can be used as a changelog entry."); + } + + if (pr.labels.filter(label => label.name.startsWith("pr-")).length !== 1) { + failures.push("Please add a `pr-` label to categorise the changelog entry."); + } + + warnings.forEach(warning => core.warning(warning)); + failures.forEach(failure => core.error(failure)); + + // Keep a single comment up to date rather than adding a new one on every run. + const marker = ""; + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number + }); + const existing = comments.find(comment => comment.body.includes(marker)); + + if (warnings.length || failures.length) { + const lines = [...failures.map(failure => `- ❌ ${failure}`), + ...warnings.map(warning => `- ⚠️ ${warning}`)]; + const commentBody = `${marker}\n### PR Checks\n\n${lines.join("\n")}`; + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body: commentBody + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + body: commentBody + }); + } + } else if (existing) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id + }); + } + + if (failures.length) { + core.setFailed(`${failures.length} check(s) failed, see the comment on the pull request.`); + } diff --git a/AGENTS.md b/AGENTS.md index 2578c5110..9d94cc146 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -380,7 +380,7 @@ Same pattern for publishers. | `app.yml` | App-level XcodeGen config | | `.swiftlint.yml` | SwiftLint rules | | `.swiftformat` | SwiftFormat rules | -| `Dangerfile.swift` | Danger PR checks | +| `.github/workflows/pr-checks.yml` | PR metadata checks (title, labels, size, screenshots) | | `Package.swift` | SPM manifest (Tools CLI) | | `localazy.json` | Localazy translation config | | `codecov.yml` | Codecov config | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b5f80ee56..b6f9f6973 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,7 +150,7 @@ Our [changelog](CHANGES.md) is automatically generated by GitHub, based on the P For Swift coding style we use [SwiftLint](https://github.com/realm/SwiftLint) to check some conventions at compile time (rules are located in the `.swiftlint.yml` file). Otherwise please have a look to [Apple Swift conventions](https://swift.org/documentation/api-design-guidelines.html#conventions). We are also using some of the conventions of [raywenderlich.com Swift style guide](https://github.com/raywenderlich/swift-style-guide). -We enforce the coding style by running checks on the CI for every PR through [Danger](Dangerfile.swift), [SwiftLint](.swiftlint.yml), [SwiftFormat](.swiftformat) and [SonarCloud](https://sonarcloud.io/project/overview?id=element-x-ios) +We enforce the coding style by running checks on the CI for every PR through [PR Checks](.github/workflows/pr-checks.yml), [SwiftLint](.swiftlint.yml), [SwiftFormat](.swiftformat) and [SonarCloud](https://sonarcloud.io/project/overview?id=element-x-ios) We also gather coverage reports on every PR through [Codecov](https://app.codecov.io/gh/element-hq/element-x-ios) and will eventually start enforcing minimums. diff --git a/Dangerfile.swift b/Dangerfile.swift deleted file mode 100644 index 9a4f3707b..000000000 --- a/Dangerfile.swift +++ /dev/null @@ -1,52 +0,0 @@ -import Danger -import Foundation - -SwiftLint.lint(.modifiedAndCreatedFiles(directory: nil), - inline: true, - configFile: nil, - strict: false, - quiet: true, - swiftlintPath: nil, - markdownAction: { _ in }) - -let danger = Danger() - -/// All of the new and modified files together. -let editedFiles = danger.git.modifiedFiles + danger.git.createdFiles - -// Warn when there is a big PR -if (danger.github.pullRequest.additions ?? 0) > 1000 { - warn("This pull request seems relatively large. Please consider splitting it into multiple smaller ones.") -} - -// Check that the PR has a description -if danger.github.pullRequest.body?.isEmpty ?? true { - warn("Please provide a description for this PR.") -} - -/// Check for screenshots on view changes -let hasChangedViews = editedFiles.contains { $0.lowercased().contains("/view") } -if hasChangedViews { - if (danger.github.pullRequest.body?.contains("user-attachments") ?? false) == false { - warn("You seem to have made changes to views. Please consider adding screenshots.") - } -} - -/// Check for pngs on resources -let hasPngs = editedFiles.contains { $0.lowercased().contains(".xcassets") && $0.lowercased().hasSuffix(".png") } -if hasPngs { - warn("You seem to have made changes to some resource images. Please consider using an SVG or PDF.") -} - -// Check for nice PR titles -let prTitle = danger.github.pullRequest.title -let fixesRegex = try! Regex("(Fixes|Fix) #\\d+") -let semanticRegex = try! Regex("\\w+\\(\\w+\\):") -if prTitle.hasSuffix("…") || prTitle.starts(with: fixesRegex) || prTitle.starts(with: semanticRegex) { - fail("Please provide a complete title that can be used as a changelog entry.") -} - -// Check for changelog tags -if danger.github.issue.labels.filter({ $0.name.hasPrefix("pr-") }).count != 1 { - fail("Please add a `pr-` label to categorise the changelog entry.") -}