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

Introduce new process for entering changelog entries prior to release PR #4385

Open
mcmire opened this issue Jun 6, 2024 · 4 comments
Open

Comments

@mcmire
Copy link
Contributor

mcmire commented Jun 6, 2024

Currently, we ask that when contributors submit a PR, they add changelog entries within the description of that PR. When a new release PR is created, the manager of the release then copies over the various changelog entries from the PRs involved in that release to various changelogs.

This process is problematic for a few reasons:

  1. Reviewers are used to using the PR description to understand the context behind the changes; they are not used to reviewing changelog entries there.
  2. If a reviewer wants to leave a suggestion for a changelog entry, it is awkward to do so in a comment on the Conversation tab. It would be more natural to attach that suggestion to a line on the Changes tab (the diff view).
  3. When preparing a release PR, it's a pain to go back through each PR to copy the changelog entries from the description into the final changelog. It would be easier to assume that they are in the correct form by the time that a release PR is created.

Putting this together, changelog entries often get missed in reviews, so in preparing a release PR, the release manager must spend time gathering context about PRs they may not have created themselves in order to reword them appropriately. Ideally, the team who introduced the change are the best people to describe that change in the changelog, and the time during which a change is being introduced is the best time to update the changelog.

To fix this, we can ask that contributors update changelogs in their PRs. However, we have another problem, which is that people still don't quite know how to do this (we find we have to make all kinds of corrections in release PRs). We should not only communicate the new process but also provide a better guide for filling out changelogs than what we have today.

Acceptance Criteria

  • Contributors have up to date instructions in the pull request template on updating changelogs.
    • The pull request template no longer contains the Changelog section and instead has a new item in the checklist at the end that reminds contributors to update the changelog in the PR.
  • Contributors have a clearer picture of how to update changelogs so that they clearly communicate API changes and make it easy for other engineers to upgrade a package to new major versions without needing to dive into PRs for more context.
    • Right now we have a little section on this in the "Releasing changes" section in the Contributing doc. I think we should extract that to a separate document at least so it's more visible.
    • If we have more time we can expound upon the existing steps, but we don't have to put too much work into it. I think providing some examples of good and not-so-good entries would probably make the biggest impact here. Mainly, changes to the API should be crystal clear (and types are part of the API too). If I'm an engineer and I'm upgrading a package, and I need to add a new argument to a constructor or pass a new argument to a function or whatever, I want to know what that is just from reading the changelog entry.
  • Contributors can consult documentation on the new process for updating changelogs alongside changes.
    • The best place for this might be the "Creating pull requests" in the Contributing doc. Currently this just asks people to fill out the PR description appropriately and specifically mentions the Changelog section in the pull request template. We can remove that second bullet point, simplify wording to focus specifically on filling out the Explanation section, and then we can add a new paragraph that reminds people to document their changes in the changelogs for the packages they've changed. We can link to the new document we created in the previous step.
  • Contributors are aware of the new changes
    • We should make a new announcement in Slack informing people of the new process.
@MajorLift
Copy link
Contributor

Is there anything keeping us from applying this practice as standard to all repos that rely on auto-changelog and the create-release-pr action? If not, would it be better to move docs/reviewing-release-prs.md and the documentation articles proposed in this ticket into contributor-docs?

@desi
Copy link
Contributor

desi commented Sep 19, 2024

We need documentation, update the PR template to include this in the check list, make a general announcement about this and push back on PRs that aren't doing this.

@mcmire
Copy link
Contributor Author

mcmire commented Nov 25, 2024

Note that in order to complete this ticket, we will need to update auto-changelog to verify that all changelog entries have associated PR links: MetaMask/auto-changelog#150. Typically, auto-changelog automatically adds these PR links in release PRs when it copies over changelogs. But if we shift the responsibility of updating changelogs from auto-changelog to contributors, it creates a bit of a chicken-and-egg situation, as contributors cannot add a PR link to a changelog entry that are adding until they create the PR. Therefore we need a verification step, otherwise there's a good chance that people will forget to add those links.

@mcmire
Copy link
Contributor Author

mcmire commented Feb 4, 2025

As stated above, this ticket is still blocked on the above ticket, however, the documentation updates are available in a branch: https://github.com/MetaMask/core/tree/add-docs-for-new-changelog-process

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

No branches or pull requests

4 participants