mirror of
https://github.com/espressif/esp-idf.git
synced 2024-10-05 20:47:46 -04:00
Add DangerJS MR review tool
Add retry link for Danger checks CI job File dangerfile.js in .gitlab directory Add notes about future development Add record to CODEOWNERS
This commit is contained in:
parent
a630f06132
commit
229d192b2a
@ -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();
|
Loading…
Reference in New Issue
Block a user