Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ [RUM-159] Categorize changes as public or internal in the CHANGELOG #2851

Merged

Conversation

RomanGaignault
Copy link
Contributor

@RomanGaignault RomanGaignault commented Jul 9, 2024

Motivation

When we created the CHANGELOG, we chose to only show public changes (bug fixes, new features, …).

In practice, this is not always respected. Sometimes, it’s not clear whether it is a public or internal change. We are a bit hesitant to show experimental features or not. Sometimes we do releases with only internal changes, and having an empty entry is not ideal.

Changes

Let’s add everything, but to help the reading, split changes into two sections (ex: public/internal changes). The generate_changelog script should handle the formating based on gitmojis.

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.64%. Comparing base (30ae145) to head (51a8e2b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2851      +/-   ##
==========================================
- Coverage   93.66%   93.64%   -0.02%     
==========================================
  Files         266      266              
  Lines        7573     7573              
  Branches     1683     1683              
==========================================
- Hits         7093     7092       -1     
- Misses        480      481       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RomanGaignault RomanGaignault marked this pull request as ready for review July 9, 2024 12:30
@RomanGaignault RomanGaignault requested a review from a team as a code owner July 9, 2024 12:30
Copy link

cit-pr-commenter bot commented Jul 9, 2024

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 161.37 KiB 161.37 KiB 0 B 0.00%
Logs 57.97 KiB 57.97 KiB 0 B 0.00%
Rum Slim 109.89 KiB 109.89 KiB 0 B 0.00%
Worker 25.21 KiB 25.21 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base Average Cpu Time (ms) Local Average Cpu Time (ms) 𝚫
addglobalcontext 0.002 0.002 0.000
addaction 0.034 0.045 0.011
adderror 0.051 0.034 -0.017
addtiming 0.001 0.001 0.000
startview 1.230 1.052 -0.179
startstopsessionreplayrecording 0.906 0.992 0.085
logmessage 0.023 0.022 -0.001
🧠 Memory Performance
Action Name Base Consumption Memory (bytes) Local Consumption Memory (bytes) 𝚫 (bytes)
addglobalcontext 22.05 KiB 20.95 KiB -1130 B
addaction 73.70 KiB 73.82 KiB 123 B
adderror 88.13 KiB 87.42 KiB -727 B
addtiming 19.17 KiB 18.72 KiB -461 B
startview 333.58 KiB 351.22 KiB 17.63 KiB
startstopsessionreplayrecording 16.50 KiB 15.68 KiB -839 B
logmessage 71.05 KiB 72.79 KiB 1.74 KiB

🔗 RealWorld

scripts/release/generate-changelog.js Outdated Show resolved Hide resolved
scripts/release/generate-changelog.js Outdated Show resolved Hide resolved
scripts/release/generate-changelog.js Show resolved Hide resolved
scripts/release/generate-changelog.js Outdated Show resolved Hide resolved
scripts/release/generate-changelog.js Outdated Show resolved Hide resolved
@RomanGaignault RomanGaignault changed the title ♻️ [RUM-159] Include internal changes in the CHANGELOG ♻️ [RUM-159] Categorize changes as public or internal in the CHANGELOG Jul 10, 2024
const START_WITH_EMOJI = /^\p{Emoji_Presentation}/u

const PUBLIC_EMOJI_PRIORITY = ['💥', '✨', '🐛', '⚡️', '📝', '⚗️']
const INTERNAL_EMOJI_PRIORITY = ['👷', '🎨', '🧪', '✅', '👌', '♻️', '🔧', '📄', '🔇', '🔊', '📦']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥜 nitpick: ‏I would reorder the table to put the emojis of the same "nature" close to one another:

['👷', '🔧', '📦', // build conf‏
'♻️', '🎨', // refactoring
'🧪', '✅',  // tests
'🔇', '🔊', // telemetry
'👌', '📄']

@RomanGaignault
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link

dd-devflow bot commented Jul 11, 2024

🚂 Branch Integration: starting soon, median merge time is 11m

Commit 51a8e2be4b will soon be integrated into staging-28.

Use /to-staging -c to cancel this operation!

dd-mergequeue bot added a commit that referenced this pull request Jul 11, 2024
) into staging-28

Integrated commit sha: 51a8e2b

Co-authored-by: roman.gaignault <[email protected]>
@dd-devflow
Copy link

dd-devflow bot commented Jul 11, 2024

🚂 Branch Integration: This commit was successfully integrated

Commit 51a8e2be4b has been merged into staging-28 in merge commit 21eb08f0d0.

Check out the triggered pipeline on Gitlab 🦊

@RomanGaignault RomanGaignault merged commit 91c814e into main Jul 11, 2024
21 checks passed
@RomanGaignault RomanGaignault deleted the roman/RUM-159-include-internal-changes-in-the-changelog branch July 11, 2024 23:47
Comment on lines +117 to +118
${internalChanges.join('\n')}
`.replace(/\(#(\d+)\)/gm, (_, id) => `([#${id}](https://github.com/DataDog/browser-sdk/pull/${id}))`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ question: ‏this is nice, but why is it done only for internal changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! What do you mean only for internal changes ? it's replacing for internal and public I guess

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry I misread! Looks fine then.


const CHANGELOG_FILE = '../../CHANGELOG.md'
const CONTRIBUTING_FILE = '../../CONTRIBUTING.md'
const PUBLIC_EMOJI_PRIORITY = ['💥', '✨', '🐛', '⚡️', '📝', '⚗️']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ The ⚡️ emoji in that list is incorrect. It's encoded as '\u26a1\ufe0f' but should be only '\u26a1'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch 👍 I will update this in the next Pr


const CHANGELOG_FILE = '../../CHANGELOG.md'
const CONTRIBUTING_FILE = '../../CONTRIBUTING.md'
const PUBLIC_EMOJI_PRIORITY = ['💥', '✨', '🐛', '⚡️', '📝', '⚗️']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 thought: ‏'⚗️' should be internal I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants