diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml new file mode 100644 index 0000000000..2741638914 --- /dev/null +++ b/.github/workflows/quality.yml @@ -0,0 +1,46 @@ +name: Code Quality Checks + +on: + pull_request: { } + push: + branches: [ main, develop ] + +# Enrich gradle.properties for CI/CD +env: + GRADLE_OPTS: -Dorg.gradle.jvmargs="-Xmx3072m -Dfile.encoding=UTF-8 -XX:+HeapDumpOnOutOfMemoryError" -XX:MaxPermSize=512m -Dkotlin.daemon.jvm.options="-Xmx2g" -Dkotlin.incremental=false + CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 2 --no-daemon + +jobs: + check: + name: Project Check Suite + runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('check-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-develop-{0}', github.sha) || format('check-{0}', github.ref) }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v3 + - name: Run code quality check suite + run: ./gradlew check $CI_GRADLE_ARG_PROPERTIES + - name: Upload reports + if: always() + uses: actions/upload-artifact@v3 + with: + name: linting-report + path: | + */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.2.0 + with: + args: "--dangerfile ./tools/danger/dangerfile-lint.js" + env: + DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }} + # Fallback for forks + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/tools/danger/dangerfile-lint.js b/tools/danger/dangerfile-lint.js new file mode 100644 index 0000000000..b0531fef9b --- /dev/null +++ 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?: "" +})) diff --git a/tools/danger/dangerfile.js b/tools/danger/dangerfile.js new file mode 100644 index 0000000000..a9be0171d7 --- /dev/null +++ b/tools/danger/dangerfile.js @@ -0,0 +1,128 @@ +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 github = danger.github +// User who has created the PR. +const user = pr.user.login +const modified = danger.git.modified_files +const created = danger.git.created_files +const 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) { + message("This pull request seems relatively large. Please consider splitting it into multiple smaller ones.") +} + +// Request a changelog for each PR +const changelogAllowList = [ + "dependabot[bot]", +] + +const requiresChangelog = !changelogAllowList.includes(user) + +if (requiresChangelog) { + const 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 that frozen classes have not been modified +const frozenClasses = [ +] + +frozenClasses.forEach(frozen => { + if (editedFiles.some(file => file.endsWith(frozen))) { + fail("Frozen class `" + frozen + "` has been modified. Please do not modify frozen class.") + } + } +) + +// Check for a sign-off +const signOff = "Signed-off-by:" + +// Please add new names following the alphabetical order. +const allowList = [ + "amitkma", + "aringenbach", + "BillCarsonFr", + "bmarty", + "Claire1817", + "dependabot[bot]", + "ericdecanini", + "fedrunov", + "Florian14", + "ganfra", + "jmartinesp", + "jonnyandrew", + "kittykat", + "langleyd", + "MadLittleMods", + "manuroe", + "mnaturel", + "onurays", + "ouchadam", + "stefanceriu", + "yostyle", +] + +const requiresSignOff = !allowList.includes(user) + +if (requiresSignOff) { + const hasPRBodySignOff = pr.body.includes(signOff) + const 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. See instructions [here](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off).") + } +} + +// Check for screenshots on view changes +const 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 +const 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 (github.requested_reviewers.users.length == 0 && !pr.draft) { + warn("Please add a reviewer to your PR.") +} + +// Check that translations have not been modified by developers +if (user != "RiotTranslateBot") { + if (editedFiles.some(file => file.endsWith("strings.xml") && !file.endsWith("values/strings.xml"))) { + fail("Some translation files have been edited. Only user `RiotTranslateBot` (i.e. translations coming from Weblate) is allowed to do that.\nPlease read more about translations management [in the doc](https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#internationalisation).") + } +}