diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 189677dfe0..e836f60e10 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -50,6 +50,7 @@ /.github/workflows/ @esp-idf-codeowners/ci /.gitlab-ci.yml @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 /.readthedocs.yml @esp-idf-codeowners/docs /CMakeLists.txt @esp-idf-codeowners/build-config diff --git a/.gitlab/ci/pre_check.yml b/.gitlab/ci/pre_check.yml index 1ba4e0f3c9..1df0ed62d0 100644 --- a/.gitlab/ci/pre_check.yml +++ b/.gitlab/ci/pre_check.yml @@ -25,6 +25,24 @@ check_pre_commit_MR: script: - 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: # 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. diff --git a/.gitlab/dangerfile.js b/.gitlab/dangerfile.js new file mode 100644 index 0000000000..d9d58ed379 --- /dev/null +++ b/.gitlab/dangerfile.js @@ -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/` 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();