-
-
Notifications
You must be signed in to change notification settings - Fork 236
[internal] Improve changelog generation script #2635
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
base: master
Are you sure you want to change the base?
[internal] Improve changelog generation script #2635
Conversation
commit: |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
60dba7e
to
04e4903
Compare
Bundle size report
|
type ChangeScope = 'docs' | 'infra' | 'public-api' | 'internal' | 'dependencies' | 'release'; | ||
type ChangelogEntryGroup = 'public-change-group-1' | 'internal-change'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename "scope" to avoid confusion; it's not a product scope, it's more of a first level of grouping.
```diff | ||
-onOpenChange: (open, event, reason) => { | ||
+onOpenChange: (open, eventDetails) => { | ||
- if (reason === 'escape-key') { | ||
+onOpenChange: (open, eventDetails) => { | ||
+ if (eventDetails.reason === 'escape-key') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct git diff output.
changelogEntries.filter((entry) => entry.scope === 'public-api'), | ||
format, | ||
); | ||
const changesList = getFormattedChangelogEntries(changelogEntries, format); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the logic to as late as we need it, keeping top-level code about high-level flow control.
prNumber, | ||
author: commit.author, | ||
group: getGroupFromLabels(labels), | ||
components: getComponentsFromLabels(labels), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To scale the generation to cover other repositories, we would probably do a change like this:
components: getComponentsFromLabels(labels), | |
subGroups: getSubGroupsFromLabels(labels), |
subGroups can be components, but also an npm packages for a given change, so the current terminology would be a bit too narrow.
What's the benefit of this? From the users' perspective, Autocomplete and Combobox are two separate components. They are imported from different paths, and they have separate documentation pages. |
@michaldudak I will wait for what you guys want to do about points 4 and 6 in https://mui-org.slack.com/archives/C02P87NQLJC/p1757017036546399?thread_ts=1756997149.279849&cid=C02P87NQLJC to update the PR to match. It depends on the product scope labeling strategy picked for the combobox. |
Simplify the changelog generation after I broke it. Help with #2615:
And one potentially controversial change:
<Autocomplete>
and<Combobox>
at the same time (my experience with this in Material UI), so it's actually simpler to have one "scope: autocomplete" +component
labels when we want to be more granular (not frequent).The alternative would be to: 1. remove this custom logic, 2. rename
component: Combobox
toscope: combobox
, and 3. renamecomponent: Autocomplete
toscope: autocomplete
.As far as I could test this PR, the generation is correct (relative to what we did for beta.3)
One step toward mui/mui-public#639