mirror of
https://github.com/espressif/esp-idf.git
synced 2024-10-05 20:47:46 -04:00
ci(danger): add dangerjs for GitHub
Add GitHub workflow for running dangerjs on pull requests. Add GitHub layout for DangerJS.
This commit is contained in:
parent
0b5ab48437
commit
7add582eb7
5
.github/dangerjs/.gitignore
vendored
Normal file
5
.github/dangerjs/.gitignore
vendored
Normal file
@ -0,0 +1,5 @@
|
|||||||
|
# Transpiled JavaScript (if any)
|
||||||
|
dist
|
||||||
|
|
||||||
|
# Installed dependencies
|
||||||
|
node_modules
|
47
.github/dangerjs/README.md
vendored
Normal file
47
.github/dangerjs/README.md
vendored
Normal file
@ -0,0 +1,47 @@
|
|||||||
|
# DangerJS pull request automatic review tool - GitHub
|
||||||
|
|
||||||
|
## Implementation
|
||||||
|
The main development is done in Espressif Gitlab project.
|
||||||
|
Espressif [GitHub project espressif/esp-idf](https://github.com/espressif/esp-idf) is only a public mirror.
|
||||||
|
|
||||||
|
Therefore, all changes and updates to DangerJS files (`.github/dangerjs`) must be made via MR in the **Gitlab** repository by Espressif engineer.
|
||||||
|
|
||||||
|
When adding a new Danger rule or updating existing one, might be a good idea to test it on the developer's fork of GitHub project. This way, the new feature can be tested using a GitHub action without concern of damaging Espressif's GitHub repository.
|
||||||
|
|
||||||
|
Danger for Espressif GitHub is implemented in TypeScript. This makes the code more readable and robust than plain JavaScript.
|
||||||
|
Compilation to JavaScript code (using `tsc`) is not necessary; Danger handles TypeScript natively.
|
||||||
|
|
||||||
|
A good practice is to store each Danger rule in a separate module, and then import these modules into the main Danger file `.github/dangerjs/dangerfile.ts` (see how this is done for currently present modules when adding a new one).
|
||||||
|
|
||||||
|
If the Danger module (new check/rule) uses an external NPM module (e.g. `axios`), be sure to add this dependency to `.github/dangerjs/package.json` and also update `.github/dangerjs/package-lock.json`.
|
||||||
|
|
||||||
|
In the GitHub action, `danger` is not installed globally (nor are its dependencies) and the `npx` call is used to start the `danger` checks in CI.
|
||||||
|
|
||||||
|
|
||||||
|
## Adding new Danger rule
|
||||||
|
For local development you can use following strategy
|
||||||
|
|
||||||
|
#### Install dependencies
|
||||||
|
```sh
|
||||||
|
cd .github/dangerjs
|
||||||
|
npm install
|
||||||
|
```
|
||||||
|
(If the IDE still shows compiler/typing errors, reload the IDE window.)
|
||||||
|
|
||||||
|
#### Add new code as needed or make updates
|
||||||
|
|
||||||
|
#### Test locally
|
||||||
|
Danger rules can be tested locally (without running the GitHub action pipeline).
|
||||||
|
To do this, you have to first export the ENV variables used by Danger in the local terminal:
|
||||||
|
|
||||||
|
```sh
|
||||||
|
export GITHUB_TOKEN='**************************************'
|
||||||
|
```
|
||||||
|
|
||||||
|
Then you can call Danger by:
|
||||||
|
```sh
|
||||||
|
cd .github/dangerjs
|
||||||
|
|
||||||
|
danger pr https://github.com/espressif/esp-idf/pull/<number_of_pull_request>
|
||||||
|
```
|
||||||
|
The result will be displayed in your terminal.
|
48
.github/dangerjs/dangerfile.ts
vendored
Normal file
48
.github/dangerjs/dangerfile.ts
vendored
Normal file
@ -0,0 +1,48 @@
|
|||||||
|
import { DangerResults } from "danger";
|
||||||
|
declare const results: DangerResults;
|
||||||
|
declare const message: (message: string, results?: DangerResults) => void;
|
||||||
|
declare const markdown: (message: string, results?: DangerResults) => void;
|
||||||
|
|
||||||
|
// Import modules with danger rules
|
||||||
|
// (Modules with checks are stored in ".github/dangerjs/<module_name>.ts". To import them, use path relative to "dangerfile.ts")
|
||||||
|
import prCommitsTooManyCommits from "./prCommitsTooManyCommits";
|
||||||
|
import prDescription from "./prDescription";
|
||||||
|
import prTargetBranch from "./prTargetBranch";
|
||||||
|
import prInfoContributor from "./prInfoContributor";
|
||||||
|
import prCommitMessage from "./prCommitMessage";
|
||||||
|
|
||||||
|
async function runDangerRules(): Promise<void> {
|
||||||
|
// Message to contributor about review and merge process
|
||||||
|
const prInfoContributorMessage: string = await prInfoContributor();
|
||||||
|
markdown(prInfoContributorMessage);
|
||||||
|
|
||||||
|
// Run danger checks
|
||||||
|
prCommitsTooManyCommits();
|
||||||
|
prDescription();
|
||||||
|
prTargetBranch();
|
||||||
|
prCommitMessage();
|
||||||
|
|
||||||
|
// Add success log if no issues
|
||||||
|
const dangerFails: number = results.fails.length;
|
||||||
|
const dangerWarns: number = results.warnings.length;
|
||||||
|
const dangerInfos: number = results.messages.length;
|
||||||
|
if (!dangerFails && !dangerWarns && !dangerInfos) {
|
||||||
|
return message("Good Job! All checks are passing!");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add retry link
|
||||||
|
addRetryLink();
|
||||||
|
}
|
||||||
|
|
||||||
|
runDangerRules();
|
||||||
|
|
||||||
|
function addRetryLink(): void {
|
||||||
|
const serverUrl: string | undefined = process.env.GITHUB_SERVER_URL;
|
||||||
|
const repoName: string | undefined = process.env.GITHUB_REPOSITORY;
|
||||||
|
const runId: string | undefined = process.env.GITHUB_RUN_ID;
|
||||||
|
|
||||||
|
const retryLinkUrl: string = `${serverUrl}/${repoName}/actions/runs/${runId}`;
|
||||||
|
const retryLink: string = `<sub>:repeat: You can re-run automatic PR checks by retrying the <a href="${retryLinkUrl}">DangerJS action</a></sub>`;
|
||||||
|
|
||||||
|
markdown(retryLink);
|
||||||
|
}
|
1999
.github/dangerjs/package-lock.json
generated
vendored
Normal file
1999
.github/dangerjs/package-lock.json
generated
vendored
Normal file
File diff suppressed because it is too large
Load Diff
18
.github/dangerjs/package.json
vendored
Normal file
18
.github/dangerjs/package.json
vendored
Normal file
@ -0,0 +1,18 @@
|
|||||||
|
{
|
||||||
|
"name": "dangerjs-github",
|
||||||
|
"description": "GitHub PR reviewing with DangerJS",
|
||||||
|
"main": "dangerfile.ts",
|
||||||
|
"keywords": [],
|
||||||
|
"author": "",
|
||||||
|
"license": "ISC",
|
||||||
|
"dependencies": {
|
||||||
|
"axios": "^1.3.3",
|
||||||
|
"danger": "^11.2.3",
|
||||||
|
"request": "^2.88.2",
|
||||||
|
"sync-request": "^6.1.0",
|
||||||
|
"typescript": "^5.0.3"
|
||||||
|
},
|
||||||
|
"devDependencies": {
|
||||||
|
"@types/node": "^18.15.11"
|
||||||
|
}
|
||||||
|
}
|
67
.github/dangerjs/prCommitMessage.ts
vendored
Normal file
67
.github/dangerjs/prCommitMessage.ts
vendored
Normal file
@ -0,0 +1,67 @@
|
|||||||
|
import { DangerDSLType, DangerResults } from "danger";
|
||||||
|
declare const danger: DangerDSLType;
|
||||||
|
declare const warn: (message: string, results?: DangerResults) => void;
|
||||||
|
|
||||||
|
interface Commit {
|
||||||
|
message: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if commit messages are sufficiently descriptive (not too short).
|
||||||
|
*
|
||||||
|
* Search for commit messages that appear to be automatically generated or temporary messages and report them.
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
export default function (): void {
|
||||||
|
const prCommits: Commit[] = danger.git.commits;
|
||||||
|
|
||||||
|
const detectRegexes: RegExp[] = [
|
||||||
|
/^Merge pull request #\d+ from .*/i, // Automatically generated message by GitHub
|
||||||
|
/^Merged .+:.+ into .+/i, // Automatically generated message by GitHub
|
||||||
|
/^Automatic merge by GitHub Action/i, // Automatically generated message by GitHub
|
||||||
|
/^Merge branch '.*' of .+ into .+/i, // Automatically generated message by GitHub
|
||||||
|
/^Create\s[a-zA-Z0-9_.-]+(\.[a-zA-Z0-9]{1,4})?(?=\s|$)/, // Automatically generated message by GitHub using UI
|
||||||
|
/^Delete\s[a-zA-Z0-9_.-]+(\.[a-zA-Z0-9]{1,4})?(?=\s|$)/, // Automatically generated message by GitHub using UI
|
||||||
|
/^Update\s[a-zA-Z0-9_.-]+(\.[a-zA-Z0-9]{1,4})?(?=\s|$)/, // Automatically generated message by GitHub using UI
|
||||||
|
/^Initial commit/i, // Automatically generated message by GitHub
|
||||||
|
/^WIP.*/i, // Message starts with prefix "WIP"
|
||||||
|
/^Cleaned.*/i, // Message starts "Cleaned", , probably temporary
|
||||||
|
/^Test:.*/i, // Message starts with "test" prefix, probably temporary
|
||||||
|
/clean ?up/i, // Message contains "clean up", probably temporary
|
||||||
|
/^[^A-Za-z0-9\s].*/, // Message starts with special characters
|
||||||
|
];
|
||||||
|
|
||||||
|
let partMessages: string[] = [];
|
||||||
|
|
||||||
|
for (const commit of prCommits) {
|
||||||
|
const commitMessage: string = commit.message;
|
||||||
|
const commitMessageTitle: string = commit.message.split("\n")[0];
|
||||||
|
|
||||||
|
// Check if the commit message matches any regex from "detectRegexes"
|
||||||
|
if (detectRegexes.some((regex) => commitMessage.match(regex))) {
|
||||||
|
partMessages.push(
|
||||||
|
`- the commit message \`${commitMessageTitle}\` appears to be a temporary or automatically generated message`
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if the commit message is not too short
|
||||||
|
const shortCommitMessageThreshold: number = 20; // commit message is considered too short below this number of characters
|
||||||
|
if (commitMessage.length < shortCommitMessageThreshold) {
|
||||||
|
partMessages.push(
|
||||||
|
`- the commit message \`${commitMessageTitle}\` may not be sufficiently descriptive`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create report
|
||||||
|
if (partMessages.length) {
|
||||||
|
partMessages.sort();
|
||||||
|
let dangerMessage = `\nSome issues found for the commit messages in this MR:\n${partMessages.join(
|
||||||
|
"\n"
|
||||||
|
)}
|
||||||
|
\nPlease consider updating these commit messages.`;
|
||||||
|
warn(dangerMessage);
|
||||||
|
}
|
||||||
|
}
|
19
.github/dangerjs/prCommitsTooManyCommits.ts
vendored
Normal file
19
.github/dangerjs/prCommitsTooManyCommits.ts
vendored
Normal file
@ -0,0 +1,19 @@
|
|||||||
|
import { DangerDSLType, DangerResults } from "danger";
|
||||||
|
declare const danger: DangerDSLType;
|
||||||
|
declare const message: (message: string, results?: DangerResults) => void;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if pull request has not an excessive numbers of commits (if squashed)
|
||||||
|
*
|
||||||
|
* @dangerjs INFO
|
||||||
|
*/
|
||||||
|
export default function (): void {
|
||||||
|
const tooManyCommitThreshold: number = 2; // above this number of commits, squash commits is suggested
|
||||||
|
const prCommits: number = danger.github.commits.length;
|
||||||
|
|
||||||
|
if (prCommits > tooManyCommitThreshold) {
|
||||||
|
return message(
|
||||||
|
`You might consider squashing your ${prCommits} commits (simplifying branch history).`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
19
.github/dangerjs/prDescription.ts
vendored
Normal file
19
.github/dangerjs/prDescription.ts
vendored
Normal file
@ -0,0 +1,19 @@
|
|||||||
|
import { DangerDSLType, DangerResults } from "danger";
|
||||||
|
declare const danger: DangerDSLType;
|
||||||
|
declare const warn: (message: string, results?: DangerResults) => void;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if pull request has has a sufficiently accurate description
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
export default function (): void {
|
||||||
|
const prDescription: string = danger.github.pr.body;
|
||||||
|
const shortPrDescriptionThreshold: number = 100; // Description is considered too short below this number of characters
|
||||||
|
|
||||||
|
if (prDescription.length < shortPrDescriptionThreshold) {
|
||||||
|
return warn(
|
||||||
|
"The PR description looks very brief, please check if more details can be added."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
58
.github/dangerjs/prInfoContributor.ts
vendored
Normal file
58
.github/dangerjs/prInfoContributor.ts
vendored
Normal file
@ -0,0 +1,58 @@
|
|||||||
|
import { DangerDSLType } from "danger";
|
||||||
|
declare const danger: DangerDSLType;
|
||||||
|
|
||||||
|
interface Contributor {
|
||||||
|
login?: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
const authorLogin = danger.github.pr.user.login;
|
||||||
|
const messageKnownContributor: string = `
|
||||||
|
***
|
||||||
|
👋 **Hi ${authorLogin}**, thank you for your another contribution to \`espressif/esp-idf\` project!
|
||||||
|
|
||||||
|
If the change is approved and passes the tests in our internal git repository, it will appear in this public Github repository on the next sync.
|
||||||
|
***
|
||||||
|
`;
|
||||||
|
|
||||||
|
const messageFirstContributor: string = `
|
||||||
|
***
|
||||||
|
👋 **Welcome ${authorLogin}**, thank you for your first contribution to \`espressif/esp-idf\` project!
|
||||||
|
|
||||||
|
📘 Please check [Contributions Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/contribute/index.html#contributions-guide) for the contribution checklist, information regarding code and documentation style, testing and other topics.
|
||||||
|
|
||||||
|
🖊️ Please also make sure you have **read and signed** the [Contributor License Agreement for espressif/esp-idf project](https://cla-assistant.io/espressif/esp-idf).
|
||||||
|
|
||||||
|
#### Pull request review and merge process you can expect
|
||||||
|
Espressif develops the ESP-IDF project in an internal repository (Gitlab). We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.
|
||||||
|
|
||||||
|
1. An internal issue has been created for the PR, we assign it to the relevant engineer
|
||||||
|
2. They review the PR and either approve it or ask you for changes or clarifications
|
||||||
|
3. Once the Github PR is approved, we synchronize it into our internal git repository
|
||||||
|
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing
|
||||||
|
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
|
||||||
|
5. If the change is approved and passes the tests it is merged into the \`master\` branch
|
||||||
|
6. On next sync from the internal git repository merged change will appear in this public Github repository
|
||||||
|
|
||||||
|
***
|
||||||
|
`;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check whether the author of the pull request is known or a first-time contributor, and add a message to the PR with information about the review and merge process.
|
||||||
|
*/
|
||||||
|
export default async function (): Promise<string> {
|
||||||
|
const contributors = await danger.github.api.repos.listContributors({
|
||||||
|
owner: danger.github.thisPR.owner,
|
||||||
|
repo: danger.github.thisPR.repo,
|
||||||
|
});
|
||||||
|
|
||||||
|
const contributorsData: Contributor[] = contributors.data;
|
||||||
|
const knownContributors: (string | undefined)[] = contributorsData.map(
|
||||||
|
(contributor: Contributor) => contributor.login
|
||||||
|
);
|
||||||
|
|
||||||
|
if (knownContributors.includes(authorLogin)) {
|
||||||
|
return messageKnownContributor;
|
||||||
|
} else {
|
||||||
|
return messageFirstContributor;
|
||||||
|
}
|
||||||
|
}
|
19
.github/dangerjs/prTargetBranch.ts
vendored
Normal file
19
.github/dangerjs/prTargetBranch.ts
vendored
Normal file
@ -0,0 +1,19 @@
|
|||||||
|
import { DangerDSLType, DangerResults } from "danger";
|
||||||
|
declare const danger: DangerDSLType;
|
||||||
|
declare const fail: (message: string, results?: DangerResults) => void;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if the target branch is "master"
|
||||||
|
*
|
||||||
|
* @dangerjs FAIL
|
||||||
|
*/
|
||||||
|
export default function (): void {
|
||||||
|
const prTargetBranch: string = danger.github?.pr?.base?.ref;
|
||||||
|
|
||||||
|
if (prTargetBranch !== "master") {
|
||||||
|
return fail(`
|
||||||
|
The target branch for this pull request should be \`master\`.\n
|
||||||
|
If you would like to add this feature to the release branch, please state this in the PR description and we will consider backporting it.
|
||||||
|
`);
|
||||||
|
}
|
||||||
|
}
|
17
.github/dangerjs/tsconfig.json
vendored
Normal file
17
.github/dangerjs/tsconfig.json
vendored
Normal file
@ -0,0 +1,17 @@
|
|||||||
|
{
|
||||||
|
"compilerOptions": {
|
||||||
|
"module": "commonjs",
|
||||||
|
"moduleResolution": "node",
|
||||||
|
"esModuleInterop": true,
|
||||||
|
"target": "es6",
|
||||||
|
"noImplicitAny": true,
|
||||||
|
"noUnusedParameters": true,
|
||||||
|
"strictNullChecks": true,
|
||||||
|
"sourceMap": true,
|
||||||
|
"removeComments": true,
|
||||||
|
"outDir": "./dist"
|
||||||
|
},
|
||||||
|
"include": [
|
||||||
|
"./*.ts"
|
||||||
|
]
|
||||||
|
}
|
41
.github/workflows/dangerjs.yml
vendored
Normal file
41
.github/workflows/dangerjs.yml
vendored
Normal file
@ -0,0 +1,41 @@
|
|||||||
|
name: DangerJS Pull Request review
|
||||||
|
|
||||||
|
on:
|
||||||
|
pull_request:
|
||||||
|
types: [opened, edited, reopened, synchronize]
|
||||||
|
branches:
|
||||||
|
- '*'
|
||||||
|
workflow_dispatch:
|
||||||
|
|
||||||
|
permissions:
|
||||||
|
actions: read
|
||||||
|
checks: read
|
||||||
|
contents: read
|
||||||
|
issues: write
|
||||||
|
pull-requests: write
|
||||||
|
security-events: read
|
||||||
|
statuses: write
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
danger-check:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
defaults:
|
||||||
|
run:
|
||||||
|
working-directory: .github/dangerjs
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v3
|
||||||
|
|
||||||
|
- name: Setup NodeJS environment
|
||||||
|
uses: actions/setup-node@v3
|
||||||
|
with:
|
||||||
|
node-version: 18
|
||||||
|
cache: npm
|
||||||
|
cache-dependency-path: .github/dangerjs/package-lock.json
|
||||||
|
|
||||||
|
- name: Install DangerJS dependencies
|
||||||
|
run: npm install
|
||||||
|
|
||||||
|
- name: Run DangerJS
|
||||||
|
run: npx danger ci --failOnErrors -v
|
||||||
|
env:
|
||||||
|
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
@ -47,6 +47,7 @@
|
|||||||
* @esp-idf-codeowners/other
|
* @esp-idf-codeowners/other
|
||||||
|
|
||||||
/.* @esp-idf-codeowners/tools
|
/.* @esp-idf-codeowners/tools
|
||||||
|
/.github/dangerjs/ @esp-idf-codeowners/ci @esp-idf-codeowners/tools
|
||||||
/.github/dependabot.yml @esp-idf-codeowners/ci
|
/.github/dependabot.yml @esp-idf-codeowners/ci
|
||||||
/.github/workflows/ @esp-idf-codeowners/ci
|
/.github/workflows/ @esp-idf-codeowners/ci
|
||||||
/.gitlab-ci.yml @esp-idf-codeowners/ci
|
/.gitlab-ci.yml @esp-idf-codeowners/ci
|
||||||
|
Loading…
Reference in New Issue
Block a user