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

Add changelog generator script #19866

Merged
merged 24 commits into from
Apr 29, 2020
Merged

Add changelog generator script #19866

merged 24 commits into from
Apr 29, 2020

Conversation

senadir
Copy link
Contributor

@senadir senadir commented Jan 24, 2020

Description

This PR is still a draft, but it makes the script available to be used before we can land on something.

Screenshots

image

Types of changes

  • Add a script changelog to wp-scripts.

@senadir senadir added [Type] Build Tooling Issues or PRs related to build tooling [Tool] WP Scripts /packages/scripts labels Jan 24, 2020
packages/scripts/README.md Outdated Show resolved Hide resolved
packages/scripts/utils/changelog/requests.js Outdated Show resolved Hide resolved
@aduth
Copy link
Member

aduth commented Jan 24, 2020

As far as I'm aware of the intentions of the @wordpress/scripts package, nothing in it should be especially specific for Gutenberg. Such scripts would make more sense (alongside existing ones) in bin.

From what I can tell, the proposed changes would not be something that anyone would run in their own projects or plugins? Would it be possible (and would there be value) in letting people use this for their own projects?

@senadir
Copy link
Contributor Author

senadir commented Jan 24, 2020

From what I can tell, the proposed changes would not be something that anyone would run in their own projects or plugins? Would it be possible (and would there be value) in letting people use this for their own projects?

I wasn’t sure if this should live in scripts or not, fwiw, the script was originally built for WooCommerce Admin, ported to WooCommerce blocks and rebuilt there, and ported again to Gutenberg, with the hope of making it public, it’s not limited to WordPress projects, and not limited to Gutenberg, but can live with a dev suit kind of tools.

@ellatrix
Copy link
Member

I tried the script and probably I'm doing something wrong with the access token. I've added:

export GH_API_TOKEN="..."

error:

(node:54229) UnhandledPromiseRejectionWarning: HttpError: Bad credentials
    at /Users/ella/gutenberg/packages/scripts/node_modules/@octokit/request/dist-node/index.js:66:23
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async getMilestoneNumber (/Users/ella/gutenberg/packages/scripts/utils/changelog/requests.js:22:15)
    at async /Users/ella/gutenberg/packages/scripts/utils/changelog/requests.js:70:27
    at async fetchAllPullRequests (/Users/ella/gutenberg/packages/scripts/utils/changelog/requests.js:69:2)
    at async make (/Users/ella/gutenberg/packages/scripts/utils/changelog/make.js:9:23)
(node:54229) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:54229) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@ellatrix
Copy link
Member

Could we not just ask for the access token when the script is running? Just like the release script?

@senadir
Copy link
Contributor Author

senadir commented Feb 11, 2020

tried the script and probably I'm doing something wrong with the access token

Are you sure the token scope is correct? public_repo admin:org and user?

Could we not just ask for the access token when the script is running? Just like the release script?

Wouldn’t this mean you need to pass the token each time, potentially creating a new one each time?
Also, you can pass it directy via the command like any other env var

@ellatrix
Copy link
Member

Ah, turned out I gave the wrong version number. I submitted "7.5" while it was expecting "Gutenberg 7.5". Should we omit "Gutenberg"?

@senadir
Copy link
Contributor Author

senadir commented Feb 11, 2020

Ah, turned out I gave the wrong version number. I submitted "7.5" while it was expecting "Gutenberg 7.5". Should we omit "Gutenberg"?

Due to the nature of the search, we need to provide the full name now, later we could use some kind of search, see @nerrad's #19866 (comment)

@ellatrix
Copy link
Member

Wouldn't it be more accurate to search commits (and relevant PRs) since between tags instead of milestones?

@ellatrix
Copy link
Member

What's holding us back here?

@youknowriad youknowriad force-pushed the add/changelog-generator branch from ffc2a38 to 82cead3 Compare April 13, 2020 08:40
@github-actions
Copy link

github-actions bot commented Apr 13, 2020

Size Change: 0 B

Total Size: 816 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.23 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.03 kB 0 B
build/block-library/editor.css 7.03 kB 0 B
build/block-library/index.js 114 kB 0 B
build/block-library/style-rtl.css 7.14 kB 0 B
build/block-library/style.css 7.14 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 179 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 27.6 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 10.9 kB 0 B
build/edit-site/style-rtl.css 5.11 kB 0 B
build/edit-site/style.css 5.11 kB 0 B
build/edit-widgets/index.js 7.5 kB 0 B
build/edit-widgets/style-rtl.css 4.67 kB 0 B
build/edit-widgets/style.css 4.66 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.6 kB 0 B
build/editor/style-rtl.css 3.27 kB 0 B
build/editor/style.css 3.27 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@youknowriad
Copy link
Contributor

I believe this command requires some refactoring (don't assume Gutenberg repo, read from package.json potentially and a good documentation about the guidelines to write PRs) in order to make it a generic command in the scripts package. I'm considering moving it to the top level (like bin/commander.js) for now in order to merge the PR as it's been used in a few releases already.

@senadir
Copy link
Contributor Author

senadir commented Apr 13, 2020

It's mainly time (didn't find time to sit and update from upstream) and there are some points that need taking care of (like special handling of some labels), the goal was to make this as generic as possible so that we can integrate with it in woocommerce/woocommerce-gutenberg-products-block and plug in zenhub, but I think that refactor will take some time so we might as well just copy the source and have it here twice for now.

The full source code: https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/master/bin/changelog

If you want to take on this @youknowriad and merge as is, and I will move the rest of the logic from the original source in a follow up.

@youknowriad youknowriad marked this pull request as ready for review April 13, 2020 10:53
@youknowriad
Copy link
Contributor

I moved the script to the bin folder for now. Let's consider merging this one.

@youknowriad
Copy link
Contributor

@aduth @senadir @nosolosw Any ideas why tests are failing on this branch. Seems related to the new octokit dependency but I can't reproduce locally 🤔

@aduth
Copy link
Member

aduth commented Apr 16, 2020

The error makes some mention of @actions/github, which is a module used by @wordpress/project-management-automation. Might try to bump the version of that, in case there's a weird conflict between these dependencies and their versions?

I'm a bit skeptical about pulling in the new @octokit/graphql, when we already have @octokit/rest:

  • More depedencies.
  • Less code consistency (maintenance burden in lessening common shared knowledge).
  • Pagination is handled quite nicely with @octokit/rest (see example)

@aduth aduth force-pushed the add/changelog-generator branch from 457c524 to 288e885 Compare April 28, 2020 18:50
@senadir senadir requested a review from chrisvanpatten as a code owner April 28, 2020 18:50
@aduth
Copy link
Member

aduth commented Apr 28, 2020

I've rebased the branch and made a handful of revisions.

Main highlights:

  • Updated release documentation.
  • Removed all prompts. By default, it requires no interaction. Requests are made anonymously (without access token) and the tool assumes to generate the notes based on the current project version. Both of these can be overridden (see updated documentation). Note that there are downsides to assuming the current project version in that this is optimized for a scenario where release notes are created after the release has been tagged. It's easy enough to override, and I expect that we could also incorporate this overriding behavior into the release tool itself at some point in the future.
  • Replaced @octokit/graphql with @octokit/rest, due to issues with Travis, concerns over maintainability of multiple API clients in the project, and the fact that only the REST API supports unauthenticated requests.

Smaller maintenance items:

  • Merged everything to a single script.
  • Type-checked by TypeScript.

@youknowriad
Copy link
Contributor

Ship it, let's iterate on follow-ups if needed.

@aduth
Copy link
Member

aduth commented Apr 28, 2020

As I go through this process of preparing release notes, I've noted a few thoughts on potential follow-up tasks:

  • If mobile app pull requests should be excluded, we could do this from the script (at least a naive pass) using the "Mobile App Compatibility" label as a condition to exclude a pull request entry.
  • Automation ideas to improve accuracy: https://github.com/WordPress/gutenberg/projects/24#card-37193288
  • If we want consistent formatting where each entry ends in a period (or doesn't end in a period), we can probably normalize this through code (title.endsWith( '.' ) ? title : title + '.').
  • Remove various common prefixes from pull request titles, and optionally use these as conditions to rearrange by type where it would otherwise fall back to the default. For example, I've seen quite a few "Various" (fallback) entries where the title is "Fix: ...".
    • Technically, we should probably want to discourage this sort of pull request titles if we don't want them in the release notes, and also better encourage people to use appropriate labels so that the fallback behavior isn't used. This is part of the "Automation ideas" above.
    • Examples: "Fix:" (treat as bug), "Docs:" (treat as documentation)
  • If we want a very specific verb phrasing (e.g. "Fixes" -> "Fix"), we could consider using a natural-language processing library (e.g. compromise project's verbs().toInfinitive()).
  • If the contents of a pull request message include specific keywords like "Fixes #XYZ", we could categorize this under "Bug Fixes" even if not labelled correctly.
  • Consider replacing common abbreviations, for improved readability of release notes ("e2e" -> "end-to-end", "IE" -> "Internet Explorer"). Optionally also for common typos or case consistency ("url" -> "URL").
  • Observing many type labels need manual updating for grouping titles (e.g. "Bug Fix" -> "Bug Fixes")
    • With the above notes about "triggers" for recategorization, maybe we shouldn't bother (or at least not rely so much on) these groupings coming from "[Type]" labels. We could have a hard-coded set of groupings, of which label could be just one hint of many.
    • Or, maybe consider updating labels where appropriate. For what it's worth, I don't think we want to change "[Type] Bug Fix" label, nor do we want to change from referring to it as "Bug Fixes" in the release notes.
    • Or, we could build-in mappings where we know a renaming will need to occur, such as with "Bug Fix" -> "Bug Fixes".

For reference, the release notes I ultimately compiled leading to the above observations: https://github.com/WordPress/gutenberg/releases/tag/v8.0.0-rc.1

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Let's keep the ball rolling.

@youknowriad youknowriad merged commit 2cf0a3d into master Apr 29, 2020
@youknowriad youknowriad deleted the add/changelog-generator branch April 29, 2020 09:54
@github-actions github-actions bot added this to the Gutenberg 8.1 milestone Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling [Type] Project Management Meta-issues related to project management of Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants