feat(release): add breaking changes to release proposal (#9196)

* add breaking changes to release proposal

* fetch from master instead

* removes stable path fallback

* fix wrong look up

Author: pabloerhard

scripts/release/changelog.js

@@ -143,18 +143,31 @@ for (const [product, scopes] of PRODUCTS) {
 
 /**
  * @param {CommitEntry[]} entries
+ * @param {CommitEntry[]} [breakingEntries]
  * @returns {ReleaseChangelog}
  */
-function createReleaseChangelog (entries) {
+function createReleaseChangelog (entries, breakingEntries = []) {
   const sections = new Map()
+  const breakingChanges = []
+  const breakingPullRequests = new Set()
   const contributors = new Set()
   const warnings = []
   let isMinor = false
 
+  for (const entry of breakingEntries) {
+    const change = parseChange(entry, { dropOtherDependencies: false })
+
+    if (change.warning) warnings.push(change.warning)
+    if (entry.author) contributors.add(entry.author)
+    if (change.pr) breakingPullRequests.add(change.pr)
+    breakingChanges.push(change)
+  }
+
   for (const entry of entries) {
     const change = parseChange(entry)
 
     if (change.drop) continue
+    if (change.pr && breakingPullRequests.has(change.pr)) continue
     if (change.warning) warnings.push(change.warning)
     if (change.category === 'Features' && !change.revert) isMinor = true
     if (entry.author) contributors.add(entry.author)
@@ -168,17 +181,18 @@ function createReleaseChangelog (entries) {
   }
 
   return {
-    markdown: renderMarkdown(sections, contributors),
+    markdown: renderMarkdown(sections, contributors, breakingChanges),
     isMinor,
     warnings,
   }
 }
 
 /**
  * @param {CommitEntry} entry
+ * @param {{ dropOtherDependencies?: boolean }} [options]
  * @returns {Change}
  */
-function parseChange (entry) {
+function parseChange (entry, options = {}) {
   const subjectWithPullRequest = parsePullRequest(entry.subject)
   const parsed = parseConventionalSubject(subjectWithPullRequest.subject)
 
@@ -194,13 +208,13 @@ function parseChange (entry) {
   }
 
   const dependency = classifyDependencyBump(parsed.scopes, parsed.subject)
-  if (dependency === 'other') {
+  if (dependency === 'other' && options.dropOtherDependencies !== false) {
     return { drop: true }
   }
 
   return {
     category: CATEGORY_BY_TYPE[parsed.type] || INTERNAL_CATEGORY,
-    product: dependency === 'production' ? DEPENDENCY_PRODUCT : selectProduct(parsed.scopes),
+    product: dependency ? DEPENDENCY_PRODUCT : selectProduct(parsed.scopes),
     subject: parsed.subject,
     pr: subjectWithPullRequest.pr,
     revert: parsed.isRevert,
@@ -321,10 +335,19 @@ function sentenceCase (subject) {
 /**
  * @param {Map<string, Change[]>} sections
  * @param {Set<string>} contributors
+ * @param {Change[]} breakingChanges
  */
-function renderMarkdown (sections, contributors) {
+function renderMarkdown (sections, contributors, breakingChanges) {
   const lines = []
 
+  if (breakingChanges.length > 0) {
+    lines.push('### Breaking Changes')
+    for (const change of breakingChanges.sort(compareChanges)) {
+      lines.push(renderChange(change))
+    }
+    lines.push('')
+  }
+
   for (const category of CATEGORY_ORDER) {
     const changes = sections.get(category)
     if (!changes?.length) continue

scripts/release/changelog.spec.js

@@ -144,6 +144,84 @@ describe('release changelog', () => {
     ].join('\n'))
   })
 
+  it('renders breaking changes at the top of the release changelog', () => {
+    const changelog = createReleaseChangelog([
+      {
+        sha: 'abc001',
+        subject: 'fix(core): keep existing behavior stable (#9001)',
+        author: '@alice',
+      },
+    ], [
+      {
+        sha: 'abc002',
+        subject: 'feat(opentelemetry)!: remove legacy propagation mode (#9002)',
+        author: '@bob',
+      },
+      {
+        sha: 'abc003',
+        subject: 'chore(deps-dev): bump eslint from 9.0.0 to 10.0.0 (#9003)',
+      },
+    ])
+
+    assert.strictEqual(changelog.markdown, [
+      '### Breaking Changes',
+      `- **Dependencies:** Bump eslint from 9.0.0 to 10.0.0 ${prLink(9003)}`,
+      `- **OpenTelemetry:** Remove legacy propagation mode ${prLink(9002)}`,
+      '',
+      '### Fixes',
+      `- **General:** Keep existing behavior stable ${prLink(9001)}`,
+      '',
+      '### Contributors',
+      '',
+      `${avatar('alice')} ${avatar('bob')}`,
+      '',
+    ].join('\n'))
+  })
+
+  it('does not promote the release to minor for breaking-only features', () => {
+    const changelog = createReleaseChangelog([
+      {
+        sha: 'abc001',
+        subject: 'fix(core): keep existing behavior stable (#9001)',
+      },
+    ], [
+      {
+        sha: 'abc002',
+        subject: 'feat(opentelemetry)!: remove legacy propagation mode (#9002)',
+      },
+    ])
+
+    assert.strictEqual(changelog.isMinor, false)
+  })
+
+  it('drops regular release note entries already listed as breaking changes', () => {
+    const changelog = createReleaseChangelog([
+      {
+        sha: 'abc001',
+        subject: 'feat(opentelemetry)!: remove legacy propagation mode (#9002)',
+      },
+      {
+        sha: 'abc002',
+        subject: 'fix(core): keep existing behavior stable (#9001)',
+      },
+    ], [
+      {
+        sha: 'abc003',
+        subject: 'feat(opentelemetry)!: remove legacy propagation mode (#9002)',
+      },
+    ])
+
+    assert.strictEqual(changelog.markdown, [
+      '### Breaking Changes',
+      `- **OpenTelemetry:** Remove legacy propagation mode ${prLink(9002)}`,
+      '',
+      '### Fixes',
+      `- **General:** Keep existing behavior stable ${prLink(9001)}`,
+      '',
+    ].join('\n'))
+    assert.strictEqual(changelog.isMinor, false)
+  })
+
   it('handles breaking markers and subjects without pull request numbers', () => {
     const changelog = createReleaseChangelog([
       {

scripts/release/proposal.js

@@ -23,6 +23,8 @@ const { checkAll } = require('./helpers/requirements')
 const tmpdir = process.env.RUNNER_TEMP || os.tmpdir()
 const main = 'master'
 const releaseLine = params[0]
+const breakingLabels = ['semver-major', 'only-land-on-next']
+const pullRequestNumberPattern = /\(#([0-9]+)\)$/
 
 // Validate release line argument.
 if (!releaseLine || releaseLine === 'help' || flags.help) {
@@ -71,11 +73,11 @@ try {
   const stableVersion = `${DD_MAJOR}.${DD_MINOR}.${DD_PATCH}`
   const isPreRelease = VERSION !== stableVersion
 
-  // Notes exclude semver-major (gated behind a flag, not user-visible).
+  // Notes list semver-major and only-land-on-next separately as breaking changes.
   // Cherry-pick includes semver-major; only only-land-on-next is fully excluded,
   // except when promoting a pre-release to stable (that's what "next" means).
   const notesDiffCmd = 'branch-diff --user DataDog --repo dd-trace-js' +
-    (isPreRelease ? '' : ' --exclude-label=semver-major --exclude-label=only-land-on-next')
+    ' --exclude-label=semver-major --exclude-label=only-land-on-next'
   const cherryPickDiffCmd = 'branch-diff --user DataDog --repo dd-trace-js' +
     (isPreRelease ? '' : ' --exclude-label=only-land-on-next')
 
@@ -94,7 +96,8 @@ try {
   // runs. It equals allMainShas[min(length, MAX_CHERRY_PICKS) - 1] regardless of
   // how many commits are already on the branch (proven by:
   // existingCherryPicked + shasToApply.length = min(allMainShas.length, MAX_CHERRY_PICKS)).
-  const upperBoundSha = allMainShas.at(Math.min(allMainShas.length, MAX_CHERRY_PICKS) - 1)
+  const upperBoundSha = allMainShas.at(Math.min(allMainShas.length, MAX_CHERRY_PICKS) - 1) ||
+    (isPreRelease ? capture(`git rev-parse v${releaseLine}.x`) : undefined)
 
   if (!upperBoundSha) {
     pass('none (already up to date)')
@@ -107,7 +110,7 @@ try {
 
   // notesShas is scoped to upperBoundSha so isMinor and release notes only reflect
   // the capped commits actually included in the proposal, not deferred ones.
-  // Excludes semver-major (gated behind a flag, not user-visible).
+  // Excludes changes that are listed in the dedicated breaking changes section.
   const notesShas = capture(`${notesDiffCmd} --format=sha --reverse v${releaseLine}.x ${upperBoundRef}`)
     .split('\n').filter(Boolean)
   const contributorBySha = getContributorsBySha(`v${releaseLine}.x`, upperBoundSha)
@@ -119,7 +122,10 @@ try {
       author: contributorBySha.get(sha),
     })
   }
-  const notes = createReleaseChangelog(notesEntries)
+  const breakingEntries = isPreRelease
+    ? getBreakingPullRequestEntries(releaseLine, upperBoundRef)
+    : []
+  const notes = createReleaseChangelog(notesEntries, breakingEntries)
   const isMinor = notes.isMinor
   const newPatch = `${releaseLine}.${DD_MINOR}.${DD_PATCH + 1}`
   const newMinor = `${releaseLine}.${DD_MINOR + 1}.0`
@@ -329,3 +335,66 @@ function getContributorsBySha (base, head) {
 
   return contributors
 }
+
+/**
+ * Semver-major changes are commonly guarded by version checks and backported to
+ * stable branches. Branch comparisons can miss them because the commits exist
+ * on both the previous and next release lines, so the vN.0.0 promotion lists
+ * merged PRs by label across the previous major cycle instead.
+ *
+ * @param {string} releaseLine
+ * @param {string} upperBoundRef
+ */
+function getBreakingPullRequestEntries (releaseLine, upperBoundRef) {
+  const previousReleaseLine = Number.parseInt(releaseLine, 10) - 1
+  const previousMajorTag = `v${previousReleaseLine}.0.0`
+  const mergedAfter = capture(`git log -1 --format=%cs ${previousMajorTag}`)
+  const mergedBefore = capture(`git show -s --format=%cs ${upperBoundRef}`)
+  const pullRequestsByNumber = new Map()
+  const includedPullRequests = getIncludedPullRequests(previousMajorTag, [`v${releaseLine}.x`, upperBoundRef])
+
+  for (const label of breakingLabels) {
+    const pullRequests = JSON.parse(capture(
+      'gh pr list --repo DataDog/dd-trace-js --state merged --limit 1000' +
+      ` --label=${label}` +
+      ` --search "base:${main} merged:>=${mergedAfter} merged:<=${mergedBefore}"` +
+      ' --json number,title,mergeCommit,author'
+    ))
+
+    for (const pullRequest of pullRequests) {
+      if (!includedPullRequests.has(pullRequest.number)) continue
+
+      pullRequestsByNumber.set(pullRequest.number, pullRequest)
+    }
+  }
+
+  return [...pullRequestsByNumber.values()].map(pullRequest => {
+    return {
+      sha: pullRequest.mergeCommit?.oid || `pull-request-${pullRequest.number}`,
+      subject: `${pullRequest.title} (#${pullRequest.number})`,
+      author: pullRequest.author?.login ? `@${pullRequest.author.login}` : undefined,
+    }
+  })
+}
+
+/**
+ * Release proposal branches are built with cherry-picks, so PR merge commits
+ * often are not ancestors of the release branch even when their changes are
+ * present. Match by the PR number preserved in the commit subject instead.
+ *
+ * @param {string} base
+ * @param {string[]} refs
+ */
+function getIncludedPullRequests (base, refs) {
+  const pullRequests = new Set()
+
+  for (const ref of refs) {
+    const subjects = capture(`git log --format=%s ${base}..${ref}`).split('\n')
+    for (const subject of subjects) {
+      const match = subject.match(pullRequestNumberPattern)
+      if (match) pullRequests.add(Number.parseInt(match[1], 10))
+    }
+  }
+
+  return pullRequests
+}