Merge pull request #6637 from vector-im/feature/bma/danger

Danger
This commit is contained in:
Benoit Marty 2022-07-29 12:55:32 +02:00 committed by GitHub
commit 59f00cd196
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 324 additions and 224 deletions

18
.github/workflows/danger.yml vendored Normal file
View file

@ -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 }}

View file

@ -29,80 +29,50 @@ jobs:
- uses: actions/checkout@v3
- name: Run knit
run: |
./gradlew knit
./gradlew knitCheck
# ktlint for all the modules
ktlint:
name: Kotlin Linter
# Check the project: ktlint, detekt, lint
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('ktlint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('ktlint-develop-{0}', github.sha) || format('ktlint-{0}', github.ref) }}
group: ${{ github.ref == 'refs/heads/main' && format('lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('lint-develop-{0}', github.sha) || format('lint-{0}', github.ref) }}
cancel-in-progress: true
steps:
- uses: actions/checkout@v3
- name: Run ktlint
run: |
./gradlew ktlintCheck --continue
- name: Run detekt
if: always()
run: |
./gradlew detekt $CI_GRADLE_ARG_PROPERTIES
- name: Run lint
# Not always, if ktlint or detekt fail, avoid running the long lint check.
run: |
./gradlew lintGplayRelease --stacktrace $CI_GRADLE_ARG_PROPERTIES
./gradlew lintFdroidRelease --stacktrace $CI_GRADLE_ARG_PROPERTIES
- name: Upload reports
if: always()
uses: actions/upload-artifact@v3
with:
name: ktlinting-report
name: linting-report
path: |
*/build/reports/ktlint/ktlint*/ktlint*.txt
- name: Handle Results
*/build/reports/**/*.*
- name: Prepare Danger
if: always()
id: ktlint-results
run: |
results="$(cat */*/build/reports/ktlint/ktlint*/ktlint*.txt */build/reports/ktlint/ktlint*/ktlint*.txt | sed -r "s/\x1B\[([0-9]{1,3}(;[0-9]{1,2})?)?[mGK]//g")"
if [ -z "$results" ]; then
echo "::set-output name=add_comment::false"
else
body="👎\`Failed${results}\`"
body="${body//'%'/'%25'}"
body="${body//$'\n'/'%0A'}"
body="${body//$'\r'/'%0D'}"
body="$( echo $body | sed 's/\/home\/runner\/work\/element-android\/element-android\//\`<br\/>\`/g')"
body="$( echo $body | sed 's/\/src\/main\/java\// 🔸 /g')"
body="$( echo $body | sed 's/im\/vector\/app\///g')"
body="$( echo $body | sed 's/im\/vector\/lib\/attachmentviewer\///g')"
body="$( echo $body | sed 's/im\/vector\/lib\/multipicker\///g')"
body="$( echo $body | sed 's/im\/vector\/lib\///g')"
body="$( echo $body | sed 's/org\/matrix\/android\/sdk\///g')"
body="$( echo $body | sed 's/\/src\/androidTest\/java\// 🔸 /g')"
echo "::set-output name=add_comment::true"
echo "::set-output name=body::$body"
fi
- name: Find Comment
if: always() && github.event_name == 'pull_request'
uses: peter-evans/find-comment@v2
id: fc
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:
issue-number: ${{ github.event.pull_request.number }}
comment-author: 'github-actions[bot]'
body-includes: Ktlint Results
- name: Add comment if needed
if: always() && github.event_name == 'pull_request' && steps.ktlint-results.outputs.add_comment == 'true'
uses: peter-evans/create-or-update-comment@v2
with:
comment-id: ${{ steps.fc.outputs.comment-id }}
issue-number: ${{ github.event.pull_request.number }}
body: |
### Ktlint Results
${{ steps.ktlint-results.outputs.body }}
edit-mode: replace
- name: Delete comment if needed
if: always() && github.event_name == 'pull_request' && steps.fc.outputs.comment-id != '' && steps.ktlint-results.outputs.add_comment == 'false'
uses: actions/github-script@v3
with:
script: |
github.issues.deleteComment({
owner: context.repo.owner,
repo: context.repo.repo,
comment_id: ${{ steps.fc.outputs.comment-id }}
})
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:
@ -122,107 +92,3 @@ jobs:
with:
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 }})
runs-on: ubuntu-latest
if: github.ref != 'refs/heads/main'
strategy:
fail-fast: false
matrix:
target: [ Gplay, Fdroid ]
# Allow all jobs on develop. Just one per PR.
concurrency:
group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}-{1}', matrix.target, github.sha) || format('apk-lint-{0}-{1}', matrix.target, 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 ${{ matrix.target }} release
run: ./gradlew clean lint${{ matrix.target }}Release --stacktrace $CI_GRADLE_ARG_PROPERTIES
- name: Upload ${{ matrix.target }} linting report
if: always()
uses: actions/upload-artifact@v3
with:
name: release-lint-report-${{ matrix.target }}
path: |
vector/build/reports/*.*
detekt:
name: Detekt Analysis
runs-on: ubuntu-latest
# Allow all jobs on main and develop. Just one per PR.
concurrency:
group: ${{ github.ref == 'refs/heads/main' && format('detekt-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('detekt-develop-{0}', github.sha) || format('detekt-{0}', github.ref) }}
cancel-in-progress: true
steps:
- uses: actions/checkout@v3
- name: Run detekt
run: |
./gradlew detekt $CI_GRADLE_ARG_PROPERTIES
- name: Upload reports
if: always()
uses: actions/upload-artifact@v3
with:
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 }}

5
.gitignore vendored
View file

@ -17,3 +17,8 @@
/fastlane/report.xml
/**/build
# Added by yarn
/package.json
/yarn.lock
/node_modules

View file

@ -1,18 +0,0 @@
# FTR: Configuration on https://travis-ci.org/github/vector-im/element-android/settings
#
# - Build only if .travis.yml is present -> On
# - Limit concurrent jobs -> Off
# - Build pushed branches -> On (build the branch)
# - Build pushed pull request -> On (build the PR after auto-merge)
#
# - Auto cancel branch builds -> On
# - Auto cancel pull request builds -> On
sudo: false
notifications:
email: false
# Just run a simple script here
script:
- ./tools/travis/check_pr.sh

View file

@ -1,3 +1,4 @@
source "https://rubygems.org"
gem "fastlane"
gem 'danger'

View file

@ -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

View file

@ -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",

1
changelog.d/6637.misc Normal file
View file

@ -0,0 +1 @@
Setup Danger to the project

102
docs/danger.md Normal file
View file

@ -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

View file

@ -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?: ""
}))

View file

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

View file

@ -1,45 +0,0 @@
#!/usr/bin/env bash
#
# Copyright 2018 New Vector Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
branch=${TRAVIS_BRANCH}
# echo ${TRAVIS_BRANCH}
# If not on develop, exit, else we cannot get the list of modified files
# It is ok to check only when on develop branch
if [[ "${branch}" -eq 'develop' ]]; then
echo "Check that a file has been added to /changelog.d"
else
echo "Not on develop branch"
exit 0
fi
# git status
listOfModifiedFiles=`git diff --name-only HEAD ${branch}`
# echo "List of modified files by this PR:"
# echo ${listOfModifiedFiles}
if [[ ${listOfModifiedFiles} = *"changelog.d"* ]]; then
echo "A file has been added to /changelog.d!"
else
echo "❌ Please add a file describing your changes in /changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog"
exit 1
fi

View file

@ -27,6 +27,7 @@ knit {
exclude '**/.gradle/**'
exclude '**/towncrier/template.md'
exclude '**/CHANGES.md'
exclude '/node_modules'
}
}

View file

@ -102,6 +102,7 @@
<!-- Wording -->
<issue id="Typos" severity="error" />
<issue id="TypographyDashes" severity="error" />
<issue id="PluralsCandidate" severity="error" />
<!-- DI -->
<issue id="JvmStaticProvidesInObjectDetector" severity="error" />

View file

@ -3018,7 +3018,7 @@
<string name="voice_message_reply_content">Voice Message (%1$s)</string>
<string name="a11y_audio_message_item">%1$s, %2$s, %3$s</string> <!-- filename, duration, file size -->
<string name="a11y_audio_playback_duration">%1$d minutes %2$d seconds</string>
<string name="a11y_audio_playback_duration" tools:ignore="PluralsCandidate">%1$d minutes %2$d seconds</string>
<string name="a11y_play_audio_message">Play %1$s</string>
<string name="a11y_pause_audio_message">Pause %1$s</string>
<string name="error_audio_message_unable_to_play">Unable to play %1$s</string>