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

[core] Fixes error in changelog generator for item sorting/padding #30088

Merged
merged 3 commits into from
Mar 28, 2022

Conversation

dimitropoulos
Copy link
Member

@dimitropoulos dimitropoulos commented Dec 6, 2021

just happened to notice this. consider if there were 10 items. That means Math.log10(10), will evaluate to 1. Then Math.ceil(1) evaluates to 1, then '9'.padStart(1, '0') evaluates to '9' when it is intended to evaluate to '09' because there are 10 items. This will then happen 10% of the time.

(not entirely sure what the right format for the PR title is for this)

just happened to notice this.  consider if there were 10 items.  That means `Math.log10(10)`, will evaluate to `1`.  Then `Math.ceil(1)` evaluates to `1`, then `'9'.padStart(1, '0')` evaluates to `'9'` when it is intended to evaluate to `'09'` because there are 10 items.
@dimitropoulos dimitropoulos requested a review from eps1lon December 6, 2021 22:41
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 6, 2021

Details of bundle changes

Generated by 🚫 dangerJS against 9815f71

@eps1lon eps1lon changed the title fixes error in changelog generator for item sorting/padding [core] Fixes error in changelog generator for item sorting/padding Dec 7, 2021
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
@danilo-leal danilo-leal added the core Infrastructure work going on behind the scenes label Feb 1, 2022
Co-authored-by: Sebastian Silbermann <[email protected]>
@mui-bot
Copy link

mui-bot commented Mar 23, 2022

No bundle size changes

Generated by 🚫 dangerJS against 73445c0

@dimitropoulos
Copy link
Member Author

oh yeah. @eps1lon @oliviertassinari @mnajdova (not sure who else would care) in case it's of any interest to you, my looking into your changelog generator ended up turning into https://github.com/Kong/changelog-generator which is being used at a few teams at Kong now (including Insomnia). There's 100% test coverage. Although the functionality diverged quite a bit, the end result is close-ish to what you all do. Just thought I'd share.

@mnajdova mnajdova merged commit a3f0060 into mui:master Mar 28, 2022
@dimitropoulos dimitropoulos deleted the patch1 branch March 28, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants