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] Generate CHANGELOG from GitHub API #3313

Merged
merged 16 commits into from Jan 24, 2022

Conversation

alexfauquette
Copy link
Member

The goal is to get a script that will not break when GitHub updates its font-end

@m4theushw
Copy link
Member

Did you consider that the results might be paginated? In #3312 it has 2 pages in the GitHub UI.

In the core repo, there's this script which you could take some inspiration: https://github.com/mui-org/material-ui/blob/master/scripts/releaseChangelog.js

It uses https://github.com/octokit/octokit.js instead of manually fetching the REST API.

@alexfauquette alexfauquette self-assigned this Dec 2, 2021
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 6, 2021
@github-actions
Copy link

github-actions bot commented Dec 6, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 21, 2021
@alexfauquette
Copy link
Member Author

The script is now able to look for PR description containing # changelog. All the text after this key will be added to the changelog generated. On the same idea, we could add break changes when we will prepare v6.

@alexfauquette
Copy link
Member Author

@m4theushw if you have a github token, the l10n can automatically update the issue when running yarn l10n --report. The token can be provided by adding GITHUB_TOKENin your env variables or adding the option --github_token 🎁

@alexfauquette alexfauquette requested review from flaviendelangle, m4theushw and DanailH and removed request for flaviendelangle and m4theushw January 7, 2022 09:24
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

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

Finally, it's the end for the inconsistencies in the CHANGELOG between each release. 😁

package.json Outdated Show resolved Hide resolved
scripts/l10n.ts Outdated
await octokit.request('PATCH /repos/{owner}/{repo}/issues/{issue_number}', {
owner: 'mui-org',
repo: 'material-ui-x',
issue_number: 3211,
Copy link
Member

Choose a reason for hiding this comment

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

Better to read it from a env var or argument.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about defining them as constants at the beginning of the file

GIT_ORGANIZATION = 'mui-org';
GIT_REPO = 'material-ui-x';
L10N_ISSUE_ID = 3211;

My concern is that someone debugging this code in 2 years will have every information set in a single file.

scripts/l10n.ts Outdated Show resolved Hide resolved
scripts/l10n.ts Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
@m4theushw m4theushw changed the title [tool] generate change-log from github API [core] Generate CHAGENLOG from GitHub API Jan 7, 2022
@m4theushw m4theushw changed the title [core] Generate CHAGENLOG from GitHub API [core] Generate CHANGELOG from GitHub API Jan 7, 2022
@m4theushw m4theushw added the core Infrastructure work going on behind the scenes label Jan 7, 2022
@alexfauquette
Copy link
Member Author

@m4theushw The dedicated PR for translation is #3588

scripts/README.md Outdated Show resolved Hide resolved
scripts/releaseChangelog.js Outdated Show resolved Hide resolved
Comment on lines 199 to 201
TODO INSERT HIGHLIGHTS

${changeLogMessages.join('\n')}
Copy link
Member

Choose a reason for hiding this comment

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

The highlights come before the package names.

Copy link
Member

@m4theushw m4theushw Jan 19, 2022

Choose a reason for hiding this comment

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

This was not addressed.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

My bad, I moved the "TODO INSERT HIGHLIGHTS" and forget the changeLogMessages

done

@mui-bot
Copy link

mui-bot commented Jan 17, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 167.8 411.4 220.1 250.34 85.485
Sort 100k rows ms 445.4 692 467.7 533.1 101.04
Select 100k rows ms 204 299 237.8 246.64 33.202
Deselect 100k rows ms 111.5 214.3 189.9 167.52 40.848

Generated by 🚫 dangerJS against 74e7179

@alexfauquette
Copy link
Member Author

I merge it because I'm doing the release this week. That will be a perfect occasion for a first test ;)

@alexfauquette alexfauquette merged commit f8f419e into mui:master Jan 24, 2022
@alexfauquette alexfauquette deleted the script branch January 24, 2022 09:09
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.

None yet

3 participants