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.") -}