forked from espressif/esp-idf
Merge branch 'feature/dangerjs' into 'master'
Add DangerJS MR review tool Closes IDF-6851, IDF-6853, and IDF-6857 See merge request espressif/esp-idf!22260
This commit is contained in:
@@ -50,6 +50,7 @@
|
|||||||
/.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
|
||||||
/.gitlab/ci/ @esp-idf-codeowners/ci
|
/.gitlab/ci/ @esp-idf-codeowners/ci
|
||||||
|
/.gitlab/dangerfile.js @esp-idf-codeowners/ci @esp-idf-codeowners/tools
|
||||||
/.pre-commit-config.yaml @esp-idf-codeowners/ci
|
/.pre-commit-config.yaml @esp-idf-codeowners/ci
|
||||||
/.readthedocs.yml @esp-idf-codeowners/docs
|
/.readthedocs.yml @esp-idf-codeowners/docs
|
||||||
/CMakeLists.txt @esp-idf-codeowners/build-config
|
/CMakeLists.txt @esp-idf-codeowners/build-config
|
||||||
|
@@ -25,6 +25,24 @@ check_pre_commit_MR:
|
|||||||
script:
|
script:
|
||||||
- python ${CI_PROJECT_DIR}/tools/ci/ci_get_mr_info.py files ${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME} | xargs pre-commit run --files
|
- python ${CI_PROJECT_DIR}/tools/ci/ci_get_mr_info.py files ${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME} | xargs pre-commit run --files
|
||||||
|
|
||||||
|
check_MR_style_dangerjs:
|
||||||
|
extends:
|
||||||
|
- .pre_check_template
|
||||||
|
image: node:14.18.0-alpine3.14
|
||||||
|
variables:
|
||||||
|
DANGER_GITLAB_API_TOKEN: ${ESPCI_TOKEN}
|
||||||
|
DANGER_GITLAB_HOST: ${GITLAB_HTTP_SERVER}
|
||||||
|
DANGER_GITLAB_API_BASE_URL: ${GITLAB_HTTP_SERVER}/api/v4
|
||||||
|
before_script:
|
||||||
|
- echo "Skip all before scripts"
|
||||||
|
script:
|
||||||
|
- set +e
|
||||||
|
- hash danger 2>/dev/null && echo "use cache" || yarn global add danger@11.2.3 --silent --skip-integrity-check --no-progress --cache-folder .yarn --global-folder .yarn-cache
|
||||||
|
- set -e
|
||||||
|
- danger ci --dangerfile=".gitlab/dangerfile.js" --failOnErrors -v
|
||||||
|
rules:
|
||||||
|
- if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
|
||||||
|
|
||||||
check_version:
|
check_version:
|
||||||
# Don't run this for feature/bugfix branches, so that it is possible to modify
|
# Don't run this for feature/bugfix branches, so that it is possible to modify
|
||||||
# esp_idf_version.h in a branch before tagging the next version.
|
# esp_idf_version.h in a branch before tagging the next version.
|
||||||
|
178
.gitlab/dangerfile.js
Normal file
178
.gitlab/dangerfile.js
Normal file
@@ -0,0 +1,178 @@
|
|||||||
|
import { danger, warn, message } from "danger"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR Title contains prefix "Draft: ... or "WIP: ...".
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrTitle() {
|
||||||
|
const mrTitle = danger.gitlab.mr.title
|
||||||
|
|
||||||
|
if (mrTitle.toUpperCase().includes("WIP") || mrTitle.toUpperCase().includes("DRAFT")) {
|
||||||
|
return warn("Please remove the `WIP:`/`Draft:` prefix from the MR name before merging this MR.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrTitle();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR Description is longer than 50 characters".
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrDescription() {
|
||||||
|
const shortMrDescriptionThreshold = 50;// MR description is considered too short below this number of characters
|
||||||
|
const mrDescription = danger.gitlab.mr.description
|
||||||
|
|
||||||
|
if (mrDescription.length < shortMrDescriptionThreshold) {
|
||||||
|
return warn("The MR description looks very brief, please check if more details can be added.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrDescription();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR Description contains mandatory section "Release notes"
|
||||||
|
*
|
||||||
|
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6852
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrReleaseNotes() {
|
||||||
|
const mrDescription = danger.gitlab.mr.description
|
||||||
|
|
||||||
|
if (!mrDescription.toUpperCase().includes("## Release notes".toUpperCase())) {
|
||||||
|
return warn("Please update the MR description, the mandatory section `Release Notes` seems to be missing.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrReleaseNotes();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR Description contains JIRA issues references
|
||||||
|
*
|
||||||
|
* Check if the associated GitHub Jira ticket has a GitHub closure reference in the commit message.
|
||||||
|
*
|
||||||
|
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6854
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrJiraLinks() {
|
||||||
|
const mrDescription = danger.gitlab.mr.description
|
||||||
|
const mrCommitMessages = danger.gitlab.commits.map(commit => commit.message);
|
||||||
|
|
||||||
|
const matchBlockRelated = mrDescription.match(/\#\# Related.*$/s); // Match MR description starting with line ## Related till the end of MR description
|
||||||
|
const testJiraLabels = /[A-Z]+-[0-9]+/.test(matchBlockRelated ? matchBlockRelated[0] : ''); // Test if pattern of Jira label "JIRA-1234" or "RDT-311" is in section Related
|
||||||
|
const ghIssueTicket = /IDFGH-[0-9]+/.test(matchBlockRelated ? matchBlockRelated[0] : ''); // Check if there is JIRA link starts with "IDFGH-*" in MR description, section "Related"
|
||||||
|
|
||||||
|
if (!mrDescription.toUpperCase().includes("## RELATED") || !testJiraLabels) { // Missing section "Related" or missing links to JIRA tickets
|
||||||
|
return warn("Please add links to JIRA issues to the MR description section `Related`.");
|
||||||
|
|
||||||
|
} else if (ghIssueTicket) { // Found JIRA ticket linked GitHub issue
|
||||||
|
if (!mrCommitMessages.includes(/Closes https:\/\/github\.com\/espressif\/esp-idf\/issues\/[0-9]+/)) { // Commit message does not contain a link to close the issue on GitHub
|
||||||
|
return warn("Please add GitHub issue closing link `Closes https://github.com/espressif/esp-idf/issues/<github-issue-number>` to the commit message.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrJiraLinks();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR has not an excessive numbers of commits (if squashed)
|
||||||
|
*
|
||||||
|
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6856.
|
||||||
|
*
|
||||||
|
* @dangerjs INFO
|
||||||
|
*/
|
||||||
|
function checkMrTooManyCommits() {
|
||||||
|
const tooManyCommitThreshold = 5 // above this number of commits, squash is recommended
|
||||||
|
const mrCommits = danger.gitlab.commits
|
||||||
|
|
||||||
|
if (mrCommits.length > tooManyCommitThreshold) {
|
||||||
|
return message(`You might consider squashing your ${mrCommits.length} commits (simplifying branch history).`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrTooManyCommits();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check commit message are descriptive enough (longer that 10 characters)
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrCommitMessagesLength() {
|
||||||
|
const shortCommitMessageThreshold = 10;// commit message is considered too short below this number of characters
|
||||||
|
const mrCommit = danger.gitlab.commits
|
||||||
|
|
||||||
|
let shortCommitMessages = [];
|
||||||
|
for (let i = 0; i < mrCommit.length; i++) {
|
||||||
|
const commitMessage = mrCommit[i].message;
|
||||||
|
|
||||||
|
if (commitMessage.length < shortCommitMessageThreshold) {
|
||||||
|
shortCommitMessages.push(`- commit message: ${commitMessage}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (shortCommitMessages.length) {
|
||||||
|
warn(`Some of your commit messages may not be sufficiently descriptive (are shorter than ${shortCommitMessageThreshold} characters):
|
||||||
|
\n${shortCommitMessages.join("")}
|
||||||
|
\nYou might consider squashing commits (simplifying branch history) or updating those short commit messages.`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrCommitMessagesLength();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if MR is too large (more than 1000 lines of changes)
|
||||||
|
*
|
||||||
|
* @dangerjs INFO
|
||||||
|
*/
|
||||||
|
function checkMrIsTooLarge() {
|
||||||
|
const bigMrLinesOfCodeThreshold = 1000
|
||||||
|
|
||||||
|
danger.git.linesOfCode()
|
||||||
|
.then((totalLines) => {
|
||||||
|
if (totalLines > bigMrLinesOfCodeThreshold) {
|
||||||
|
return message(`This MR seems to be quiet large (total lines of code: ${totalLines}), you might consider splitting it into smaller MRs`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
checkMrIsTooLarge();
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if documentation needs translation labels
|
||||||
|
*
|
||||||
|
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6855.
|
||||||
|
*
|
||||||
|
* @dangerjs WARN
|
||||||
|
*/
|
||||||
|
function checkMrNeedsTranlation() {
|
||||||
|
const mrLabels = danger.gitlab.mr.labels
|
||||||
|
const changesInDocsEN = /docs\/en/.test(danger.git.modified_files ? danger.git.modified_files[0] : ''); // Test if changes in directory "docs/EN"
|
||||||
|
const changesInDocsCH = /docs\/zh_CN/.test(danger.git.modified_files ? danger.git.modified_files[0] : ''); // Test if changes in directory "docs/CH"
|
||||||
|
|
||||||
|
// Only English docs has been changed
|
||||||
|
if (changesInDocsEN && !changesInDocsCH) {
|
||||||
|
if (!mrLabels.includes("needs translation: CN")) {
|
||||||
|
return warn("The updated documentation will need to be translated into Chinese, please add the MR label `needs translation: CN`");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Only Chineese docs has been changed
|
||||||
|
if (!changesInDocsEN && changesInDocsCH) {
|
||||||
|
if (!mrLabels.includes("needs translation: EN")) {
|
||||||
|
return warn("The updated documentation will need to be translated into English, please add the MR label `needs translation: EN`");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checkMrNeedsTranlation();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Add a link to manual retry a DangerJS job (without committing to the repository)
|
||||||
|
*
|
||||||
|
* @dangerjs MARKDOWN
|
||||||
|
*/
|
||||||
|
function addRetryLink() {
|
||||||
|
const retryLink = `${process.env.DANGER_GITLAB_HOST}/${process.env.CI_PROJECT_PATH}/-/jobs/${process.env.CI_JOB_ID}`
|
||||||
|
return markdown(`***\n#### :repeat: If you want to run these checks again, please retry this [DangerJS job](${retryLink})\n***`);
|
||||||
|
}
|
||||||
|
addRetryLink();
|
Reference in New Issue
Block a user