diff --git a/.github/chainguard/self.check-pull-request-size.sts.yaml b/.github/chainguard/self.check-pull-request-size.sts.yaml new file mode 100644 index 00000000000..823ffe3733e --- /dev/null +++ b/.github/chainguard/self.check-pull-request-size.sts.yaml @@ -0,0 +1,11 @@ +issuer: https://token.actions.githubusercontent.com + +subject: repo:DataDog/dd-trace-java:pull_request + +claim_pattern: + event_name: pull_request + job_workflow_ref: DataDog/dd-trace-java/\.github/workflows/check-pull-request-size\.yaml@refs/(pull/[0-9]+/merge|heads/.+) + +permissions: + issues: write + pull_requests: write diff --git a/.github/scripts/count-added-lines.sh b/.github/scripts/count-added-lines.sh new file mode 100755 index 00000000000..4e8d8a14c91 --- /dev/null +++ b/.github/scripts/count-added-lines.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Counts the lines added from the merge base of two revisions, excluding moved lines and lock files. +# Usage: count-added-lines.sh [head] +# +# Moved lines (blocks of at least 20 alphanumeric characters deleted and added elsewhere, even +# re-indented or in another file) are told apart from added lines using diff colors: +# added lines are painted green and moved lines blue, then only the green lines are counted. +set -euo pipefail + +BASE=${1:?Usage: $0 [head]} +HEAD=${2:-HEAD} + +git \ + -c color.diff.meta=bold \ + -c color.diff.new=green \ + -c color.diff.newMoved=blue \ + diff \ + --color=always \ + --color-moved=blocks \ + --color-moved-ws=allow-indentation-change \ + --ws-error-highlight=none \ + --merge-base \ + --find-renames \ + --no-ext-diff \ + "$BASE" "$HEAD" \ + -- . ':(exclude,glob)**/gradle.lockfile' | + awk '/^\033\[32m\+/ { count++ } END { print count + 0 }' diff --git a/.github/workflows/README.md b/.github/workflows/README.md index d1f8e2af197..2e2048eace7 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -40,6 +40,15 @@ _Actions:_ _Recovery:_ Update the pull request or add a comment to trigger the action again. +### check-pull-request-size [🔗](check-pull-request-size.yaml) + +_Trigger:_ When creating or updating a pull request targeting `master` or a patch release (`release/vM.N.x`) branch, or when labels are updated. Draft pull requests are skipped. + +_Action:_ Fail the pull request if it adds 1000 lines or more, then comment asking to split it into smaller stacked pull requests. +Deleted lines, moved code (detected using `git diff --color-moved`), and `gradle.lockfile` files are not counted. + +_Recovery:_ Split the pull request into smaller stacked pull requests. To override this check entirely, add the `tag: override pr size check` label to the PR. Remove the label to re-enable the check. + ### enforce-datadog-merge-queue [🔗](enforce-datadog-merge-queue.yaml) _Trigger:_ When creating or updating a pull request, or when a pull request is added to GitHub merge queue. diff --git a/.github/workflows/check-pull-request-size.yaml b/.github/workflows/check-pull-request-size.yaml new file mode 100644 index 00000000000..31b0433b49e --- /dev/null +++ b/.github/workflows/check-pull-request-size.yaml @@ -0,0 +1,110 @@ +name: Check pull request size +on: + pull_request: + types: [opened, reopened, edited, ready_for_review, labeled, unlabeled, synchronize] + branches: + - master + - "release/v*" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + check_pull_request_size: + name: Check pull request size + # Skip draft pull requests + if: ${{ !github.event.pull_request.draft }} + permissions: + contents: read # Required to check out the pull request changes + id-token: write # Required for OIDC token federation + runs-on: ubuntu-latest + steps: + - name: Checkout pull request changes + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 + with: + fetch-depth: 2 # Fetch the pull request merge commit and its parents + filter: blob:none + sparse-checkout: .github/scripts + persist-credentials: false + - name: Count added lines + id: count-added-lines + run: | + # The first parent of the pull request merge commit is the base branch + ADDED_LINES=$(.github/scripts/count-added-lines.sh HEAD^1 HEAD) + echo "Added lines: $ADDED_LINES" + echo "added_lines=$ADDED_LINES" >> "$GITHUB_OUTPUT" + - name: Get GitHub Token via dd-octo-sts + id: generate-token + uses: DataDog/dd-octo-sts-action@c85802a58e202b7e764b5441e4378c76043c8108 # v1.0.5 + with: + scope: DataDog/dd-trace-java + policy: self.check-pull-request-size + - name: Check pull request size + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # 9.0.0 + env: + ADDED_LINES: ${{ steps.count-added-lines.outputs.added_lines }} + MAX_ADDED_LINES: 1000 + with: + github-token: ${{ steps.generate-token.outputs.token }} + script: | + const managedMarker = '' + const overrideLabel = 'tag: override pr size check' + const addedLines = parseInt(process.env.ADDED_LINES, 10) + const maxAddedLines = parseInt(process.env.MAX_ADDED_LINES, 10) + + // Look for the previous managed comment + const comments = await github.paginate(github.rest.issues.listComments, { + issue_number: context.payload.pull_request.number, + owner: context.repo.owner, + repo: context.repo.repo + }) + const existingComment = comments.find(comment => comment.body?.includes(managedMarker)) + + const deleteManagedComment = async () => { + if (existingComment) { + await github.rest.issues.deleteComment({ + comment_id: existingComment.id, + owner: context.repo.owner, + repo: context.repo.repo + }) + } + } + + // Check for override label — skip the check if label present + const labels = context.payload.pull_request.labels.map(l => l.name) + if (labels.includes(overrideLabel)) { + await deleteManagedComment() + console.log(`${overrideLabel} label detected — skipping the check.`) + return + } + + if (addedLines < maxAddedLines) { + await deleteManagedComment() + console.log(`Pull request adds ${addedLines} lines, below the ${maxAddedLines} lines limit.`) + return + } + + const body = `**❌ Pull Request Too Large**\n\n` + + `This pull request adds **${addedLines}** lines while the limit is **${maxAddedLines}** lines.\n` + + `Deleted lines, moved code, and \`gradle.lockfile\` files are not counted.\n\n` + + `Please split it into smaller stacked pull requests, each one targeting the branch of the previous one. ` + + `See the [pull request size guidelines](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#pull-request-size) for more details.\n\n` + + `If this PR cannot be split, exceptionnaly add the \`${overrideLabel}\` label to bypass this workflow.\n\n` + + managedMarker + if (existingComment) { + await github.rest.issues.updateComment({ + comment_id: existingComment.id, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } else { + await github.rest.issues.createComment({ + issue_number: context.payload.pull_request.number, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } + core.setFailed(`Pull request adds ${addedLines} lines, above the ${maxAddedLines} lines limit. See PR comment for details. To bypass this workflow, add the '${overrideLabel}' label.`) diff --git a/AGENTS.md b/AGENTS.md index 065d5f9b77c..487a73657e1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,7 @@ docs/ Developer documentation (see below) ## PR conventions - Title: imperative verb sentence describing user-visible change (e.g. "Fix span sampling rule parsing") +- Size: keep PRs small (under 1000 added lines), split larger changes into stacked PRs. - Labels: always add `tag: ai generated` and at least one `comp:` or `inst:` label + one `type:` label - Use `tag: no release notes` for internal/refactoring changes - Open as draft first, convert to ready when reviewable diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1ea7d895e36..ad01f872b8a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -104,6 +104,17 @@ and include them in the same PR. This applies to changes in build scripts and `gradle/libs.versions.toml`. Follow the [lockfile update workflow](docs/how_to_work_with_gradle.md#updating-lock-files) before requesting review. +### Pull request size + +Pull requests should add less than 1000 lines to remain reviewable. +Deleted lines, moved code, and `gradle.lockfile` files are not counted. +Run `.github/scripts/count-added-lines.sh origin/master HEAD` to check the added line count of your branch. + +Split larger changes into stacked pull requests: each pull request targets the branch of the previous one, and they are reviewed and merged in order. + +>[!NOTE] +> If a pull request cannot be split (e.g. a large test migration), add the [`tag: override pr size check` label](#labels) to bypass the check. + ### Title format Pull request titles should briefly describe the proposed changes in a way that makes sense for the users. @@ -140,6 +151,7 @@ Labels are used to not only categorize but also alter the continuous integration * Refactoring pull requests * CI and build tools improvements * Minor changes like typo +* `tag: override pr size check` to bypass [the pull request size check](#pull-request-size) when a pull request cannot be split * [The `run-tests:` category](https://github.com/DataDog/dd-trace-java/labels?q=run-tests%3A) to run continuous integration tests on a specific JVM in case of JVM specific changes * `run-tests: flaky` to run the flaky tests on continuous integration as they are disabled by default