Setup Danger to Element Android project.

2024-11-23 14:48:21 +08:00 · 2022-07-29 10:00:49 +02:00 · 2022-07-29 10:00:49 +02:00 · 56691f613e
commit 56691f613e
parent a52a2504ef
9 changed files with 333 additions and 51 deletions
--- a/.github/workflows/danger.yml
+++ b/.github/workflows/danger.yml
@ -0,0 +1,18 @@
+name: Danger CI
+
+on: [pull_request]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    name: Danger
+    steps:
+      - uses: actions/checkout@v3
+      - run: |
+          npm install --save-dev @babel/plugin-transform-flow-strip-types
+      - name: Danger
+        uses: danger/danger-js@11.1.1
+        with:
+          args: "--dangerfile tools/danger/dangerfile.js"
+        env:
+          DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}
--- a/.github/workflows/quality.yml
+++ b/.github/workflows/quality.yml
@ -103,6 +103,19 @@ jobs:
              repo: context.repo.repo,
              comment_id: ${{ steps.fc.outputs.comment-id }}
            })
+      - name: Prepare Danger
+        if: always()
+        run: |
+          npm install --save-dev @babel/core
+          npm install --save-dev @babel/plugin-transform-flow-strip-types
+          yarn add danger-plugin-lint-report --dev
+      - name: Danger lint
+        if: always()
+        uses: danger/danger-js@11.1.1
+        with:
+          args: "--dangerfile tools/danger/dangerfile-lint.js"
+        env:
+          DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}

  # Gradle dependency analysis using https://github.com/autonomousapps/dependency-analysis-android-gradle-plugin
  dependency-analysis:
@ -123,34 +136,6 @@ jobs:
          name: dependency-analysis
          path: build/reports/dependency-check-report.html

-  # Lint for main module
-  android-lint:
-    name: Android Linter
-    runs-on: ubuntu-latest
-    # Allow all jobs on main and develop. Just one per PR.
-    concurrency:
-      group: ${{ github.ref == 'refs/heads/main' && format('android-lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('android-lint-develop-{0}', github.sha) || format('android-lint-{0}', github.ref) }}
-      cancel-in-progress: true
-    steps:
-      - uses: actions/checkout@v3
-      - uses: actions/cache@v3
-        with:
-          path: |
-            ~/.gradle/caches
-            ~/.gradle/wrapper
-          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}
-          restore-keys: |
-            ${{ runner.os }}-gradle-
-      - name: Lint analysis
-        run: ./gradlew clean :vector:lint --stacktrace $CI_GRADLE_ARG_PROPERTIES
-      - name: Upload reports
-        if: always()
-        uses: actions/upload-artifact@v3
-        with:
-          name: lint-report
-          path: |
-            vector/build/reports/*.*
-
  # Lint for Gplay and Fdroid release APK
  apk-lint:
    name: Lint APK (${{ matrix.target }})
@ -183,6 +168,19 @@ jobs:
          name: release-lint-report-${{ matrix.target }}
          path: |
            vector/build/reports/*.*
+      - name: Prepare Danger
+        if: always()
+        run: |
+          npm install --save-dev @babel/core
+          npm install --save-dev @babel/plugin-transform-flow-strip-types
+          yarn add danger-plugin-lint-report --dev
+      - name: Danger lint
+        if: always()
+        uses: danger/danger-js@11.1.1
+        with:
+          args: "--dangerfile tools/danger/dangerfile-lint.js"
+        env:
+          DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}

  detekt:
    name: Detekt Analysis
@ -203,26 +201,16 @@ jobs:
          name: detekt-report
          path: |
            */build/reports/detekt/detekt.html
-
-#  towncrier:
-#    name: Towncrier check
-#    runs-on: ubuntu-latest
-#    if: github.event_name == 'pull_request' && github.head_ref == 'develop'
-#    steps:
-#      - uses: actions/checkout@v3
-#      - name: Set up Python 3.8
-#        uses: actions/setup-python@v4
-#        with:
-#          python-version: 3.8
-#      - name: Install towncrier
-#        run: |
-#          python3 -m pip install towncrier
-#      - name: Run towncrier
-#        # Fetch the pull request' base branch so towncrier will be able to
-#        # compare the current branch with the base branch.
-#        # Source: https://github.com/actions/checkout/#fetch-all-branches.
-#        run: |
-#          git fetch --no-tags origin +refs/heads/${BASE_BRANCH}:refs/remotes/origin/${BASE_BRANCH}
-#          towncrier check --compare-with origin/${BASE_BRANCH}
-#      env:
-#        BASE_BRANCH: ${{ github.base_ref }}
+      - name: Prepare Danger
+        if: always()
+        run: |
+          npm install --save-dev @babel/core
+          npm install --save-dev @babel/plugin-transform-flow-strip-types
+          yarn add danger-plugin-lint-report --dev
+      - name: Danger lint
+        if: always()
+        uses: danger/danger-js@11.1.1
+        with:
+          args: "--dangerfile tools/danger/dangerfile-lint.js"
+        env:
+          DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}
--- a/.gitignore
+++ b/.gitignore
@ -17,3 +17,8 @@
 /fastlane/report.xml

 /**/build
+
+# Added by yarn
+/package.json
+/yarn.lock
+/node_modules
--- a/1
+++ b/1
@ -1,3 +1,4 @@
 source "https://rubygems.org"

 gem "fastlane"
+gem 'danger'
--- a/Gemfile.lock
+++ b/Gemfile.lock
@ -24,10 +24,29 @@ GEM
      aws-eventstream (~> 1, >= 1.0.2)
    babosa (1.0.4)
    claide (1.0.3)
+    claide-plugins (0.9.2)
+      cork
+      nap
+      open4 (~> 1.3)
    colored (1.2)
    colored2 (3.1.2)
    commander (4.6.0)
      highline (~> 2.0.0)
+    cork (0.3.0)
+      colored2 (~> 3.1)
+    danger (8.6.1)
+      claide (~> 1.0)
+      claide-plugins (>= 0.9.2)
+      colored2 (~> 3.1)
+      cork (~> 0.1)
+      faraday (>= 0.9.0, < 2.0)
+      faraday-http-cache (~> 2.0)
+      git (~> 1.7)
+      kramdown (~> 2.3)
+      kramdown-parser-gfm (~> 1.0)
+      no_proxy_fix
+      octokit (~> 4.7)
+      terminal-table (>= 1, < 4)
    declarative (0.0.20)
    digest-crc (0.6.3)
      rake (>= 12.0.0, < 14.0.0)
@ -52,6 +71,8 @@ GEM
    faraday-em_http (1.0.0)
    faraday-em_synchrony (1.0.0)
    faraday-excon (1.1.0)
+    faraday-http-cache (2.4.0)
+      faraday (>= 0.8)
    faraday-httpclient (1.0.1)
    faraday-net_http (1.0.1)
    faraday-net_http_persistent (1.2.0)
@ -98,6 +119,8 @@ GEM
      xcpretty (~> 0.3.0)
      xcpretty-travis-formatter (>= 0.0.3)
    gh_inspector (1.1.3)
+    git (1.11.0)
+      rchardet (~> 1.8)
    google-apis-androidpublisher_v3 (0.8.0)
      google-apis-core (>= 0.4, < 2.a)
    google-apis-core (0.4.0)
@ -143,17 +166,28 @@ GEM
    jmespath (1.4.0)
    json (2.5.1)
    jwt (2.2.3)
+    kramdown (2.4.0)
+      rexml
+    kramdown-parser-gfm (1.1.0)
+      kramdown (~> 2.0)
    memoist (0.16.2)
    mini_magick (4.11.0)
    mini_mime (1.1.0)
    multi_json (1.15.0)
    multipart-post (2.0.0)
    nanaimo (0.3.0)
+    nap (1.1.0)
    naturally (2.2.1)
+    no_proxy_fix (0.1.2)
+    octokit (4.25.1)
+      faraday (>= 1, < 3)
+      sawyer (~> 0.9)
+    open4 (1.3.4)
    os (1.1.1)
    plist (3.6.0)
    public_suffix (4.0.6)
    rake (13.0.6)
+    rchardet (1.8.0)
    representable (3.1.1)
      declarative (< 0.1.0)
      trailblazer-option (>= 0.1.1, < 0.2.0)
@ -163,6 +197,9 @@ GEM
    rouge (2.0.7)
    ruby2_keywords (0.0.5)
    rubyzip (2.3.2)
+    sawyer (0.9.2)
+      addressable (>= 2.3.5)
+      faraday (>= 0.17.3, < 3)
    security (0.1.3)
    signet (0.15.0)
      addressable (~> 2.3)
@ -200,9 +237,11 @@ GEM
      xcpretty (~> 0.2, >= 0.0.7)

 PLATFORMS
+  universal-darwin-21
  x86_64-darwin-20

 DEPENDENCIES
+  danger
  fastlane

 BUNDLED WITH
--- a/build.gradle
+++ b/build.gradle
@ -126,6 +126,11 @@ allprojects {
        enableExperimentalRules = true
        // display the corresponding rule
        verbose = true
+        reporters {
+            reporter(org.jlleitschuh.gradle.ktlint.reporter.ReporterType.PLAIN)
+            // To have XML report for Danger
+            reporter(org.jlleitschuh.gradle.ktlint.reporter.ReporterType.CHECKSTYLE)
+        }
        disabledRules = [
                // TODO Re-enable these 4 rules after reformatting project
                "indent",
--- a/docs/danger.md
+++ b/docs/danger.md
@ -0,0 +1,102 @@
+## Danger
+
+<!--- TOC -->
+
+* [What does danger checks](#what-does-danger-checks)
+  * [PR check](#pr-check)
+  * [Quality check](#quality-check)
+* [Setup](#setup)
+* [Run danger locally](#run-danger-locally)
+* [Danger user](#danger-user)
+* [Useful links](#useful-links)
+
+<!--- END -->
+
+## What does danger checks
+
+### PR check
+
+See the [dangerfile](../tools/danger/dangerfile.js). If you add rules in the dangerfile, please update the list below!
+
+Here are the checks that Danger does so far:
+
+- PR description is not empty
+- Big PR got a warning to recommend to split
+- PR contains a file for towncrier and extension is checked
+- PR contains a Sign-Off, with exception for Element employee contributors
+- PR with change on layout should include screenshot in the description
+- PR which adds png file warn about the usage of vector drawables
+- non draft PR should have a reviewer
+
+### Quality check
+
+After all the checks that generate checkstyle XML report, such as Ktlint, lint, or Detekt, Danger is run with this [dangerfile](../tools/danger/dangerfile-lint.js), in order to post comments to the PR with the detected error and warnings.
+
+To run locally, you will have to install the plugin `danger-plugin-lint-report` using:
+
+```shell
+yarn add danger-plugin-lint-report --dev
+```
+
+## Setup
+
+This operation should not be necessary, since Danger is already setup for the project.
+
+To setup danger to the project, run:
+
+```shell
+bundle exec danger init
+```
+
+## Run danger locally
+
+When modifying the [dangerfile](../tools/danger/dangerfile.js), you can check it by running Danger locally.
+
+To run danger locally, install it and run:
+
+```shell
+bundle exec danger pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
+```
+
+For instance:
+
+```shell
+bundle exec danger pr https://github.com/vector-im/element-android/pull/6637 --dangerfile=./tools/danger/dangerfile.js
+```
+
+We may need to create a GitHub token to have less API rate limiting, and then set the env var:
+
+```shell
+export DANGER_GITHUB_API_TOKEN='YOUR_TOKEN'
+```
+
+Swift and Kotlin (just in case)
+
+```shell
+bundle exec danger-swift pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
+bundle exec danger-kotlin pr <PR_URL> --dangerfile=./tools/danger/dangerfile.js
+```
+
+## Danger user
+
+To let Danger check all the PRs, including PRs form forks, a GitHub account have been created:
+- login: ElementBot
+- password: Stored on Passbolt
+- GitHub token: A token with limited access has been created and added to the repository https://github.com/vector-im/element-android as secret DANGER_GITHUB_API_TOKEN. This token is not saved anywhere else. In case of problem, just delete it and create a new one, then update the secret.
+
+## Useful links
+
+- https://danger.systems/
+- https://danger.systems/js/
+- https://danger.systems/js/guides/getting_started.html
+- https://danger.systems/js/reference.html
+- https://github.com/danger/awesome-danger
+
+Some danger files to get inspired from
+
+- https://github.com/artsy/emission/blob/master/dangerfile.ts
+- https://github.com/facebook/react-native/blob/master/bots/dangerfile.js
+- https://github.com/apollographql/apollo-client/blob/master/config/dangerfile.ts
+- https://github.com/styleguidist/react-styleguidist/blob/master/dangerfile.js
+- https://github.com/storybooks/storybook/blob/master/dangerfile.js
+- https://github.com/ReactiveX/rxjs/blob/master/dangerfile.js
--- a/tools/danger/dangerfile-lint.js
+++ b/tools/danger/dangerfile-lint.js
@ -0,0 +1,29 @@
+import { schedule } from 'danger'
+
+/**
+ * Ref and documentation: https://github.com/damian-burke/danger-plugin-lint-report
+ * This file will check all the error in XML Checkstyle format.
+ * It covers, lint, ktlint, and detekt errors
+ */
+
+const reporter = require("danger-plugin-lint-report")
+schedule(reporter.scan({
+    /**
+     * File mask used to find XML checkstyle reports.
+     */
+    fileMask: "**/reports/**/**.xml",
+    /**
+     * If set to true, the severity will be used to switch between the different message formats (message, warn, fail).
+     */
+    reportSeverity: true,
+    /**
+     * If set to true, only issues will be reported that are contained in the current changeset (line comparison).
+     * If set to false, all issues that are in modified files will be reported.
+     */
+    requireLineModification: false,
+    /**
+     * Optional: Sets a prefix foreach violation message.
+     * This can be useful if there are multiple reports being parsed to make them distinguishable.
+     */
+     // outputPrefix?: ""
+}))
--- a/tools/danger/dangerfile.js
+++ b/tools/danger/dangerfile.js
@ -0,0 +1,95 @@
+const {danger, warn} = require('danger')
+
+/**
+ * Note: if you update the checks in this file, please also update the file ./docs/danger.md
+ */
+
+// Useful to see what we got in danger object
+// warn(JSON.stringify(danger))
+
+const pr = danger.github.pr
+const modified = danger.git.modified_files
+const created = danger.git.created_files
+let editedFiles = [...modified, ...created]
+
+// Check that the PR has a description
+if (pr.body.length == 0) {
+    warn("Please provide a description for this PR.")
+}
+
+// Warn when there is a big PR
+if (editedFiles.length > 50) {
+    warn("This pull request seems relatively large. Please consider splitting it into multiple smaller ones.")
+}
+
+// Request a changelog for each PR
+let changelogFiles = editedFiles.filter(file => file.startsWith("changelog.d/"))
+
+if (changelogFiles.length == 0) {
+    warn("Please add a changelog. See instructions [here](https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog)")
+} else {
+    const validTowncrierExtensions = [
+        "bugfix",
+        "doc",
+        "feature",
+        "misc",
+        "sdk",
+        "wip",
+    ]
+    if (!changelogFiles.every(file => validTowncrierExtensions.includes(file.split(".").pop()))) {
+        fail("Invalid extension for changelog. See instructions [here](https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog)")
+    }
+}
+
+// Check for a sign-off
+const signOff = "Signed-off-by:"
+
+// Please add new names following the alphabetical order.
+const allowList = [
+    "aringenbach",
+    "BillCarsonFr",
+    "bmarty",
+    "Claire1817",
+    "ericdecanini",
+    "fedrunov",
+    "Florian14",
+    "ganfra",
+    "jmartinesp",
+    "langleyd",
+    "MadLittleMods",
+    "manuroe",
+    "mnaturel",
+    "onurays",
+    "ouchadam",
+    "stefanceriu",
+    "yostyle",
+]
+
+let requiresSignOff = !allowList.includes(pr.user.login)
+
+if (requiresSignOff) {
+    let hasPRBodySignOff = pr.body.includes(signOff)
+    let hasCommitSignOff = danger.git.commits.every(commit => commit.message.includes(signOff))
+    if (!hasPRBodySignOff && !hasCommitSignOff) {
+        fail("Please add a sign-off to either the PR description or to the commits themselves.")
+    }
+}
+
+// Check for screenshots on view changes
+let hasChangedViews = editedFiles.filter(file => file.includes("/layout")).length > 0
+if (hasChangedViews) {
+    if (!pr.body.includes("user-images")) {
+        warn("You seem to have made changes to views. Please consider adding screenshots.")
+    }
+}
+
+// Check for pngs on resources
+let hasPngs = editedFiles.filter(file => file.toLowerCase().endsWith(".png")).length > 0
+if (hasPngs) {
+    warn("You seem to have made changes to some images. Please consider using an vector drawable.")
+}
+
+// Check for reviewers
+if (pr.requested_reviewers.length == 0 && !pr.draft) {
+    fail("Please add a reviewer to your PR.")
+}