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

Process: Add checklist item about API changes in a PR #21499

Closed
wants to merge 1 commit into from

Conversation

akirk
Copy link
Member

@akirk akirk commented Apr 9, 2020

Description

To avoid situations where data corruption can be a result of changing API (see #18616 which started storing escaped titles in the database), we need to better aware of such changes. Most API changes will be without side-effects but without awareness of potential breaking changes, those that do have side-effects might be recognized too late.

By flagging API changes, they can be reviewed in bulk before a release, and the correct measures can be taken, for examply by adding a Changelog entry.

Types of changes

Adds a new checklist item to the PR template:

[ ] My code adds or changes API interaction.

Checklist:

(none apply since this is not a code change)

Proposal in change of process

We should create a new status label Changes API so that we can review API changes before releases. There should also be new labels Requires Changelog Entry and Has Changelog Entry which can be used in such a case but might be useful in further scenarios when we want to ensure that important changes are not forgotten in changelogs.

@youknowriad
Copy link
Contributor

While I completely agree on the principle, experience had shown that these templates are not effective and shouldn't be considered as sufficient. This is to say that I'm ok with this addition but I don't expect it to have meaningful impact.

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

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

I'll approve because it can't hurt adding an extra prompt for people to consider.
Though I'll echo @youknowriad comment that the checklist items are not too effective.

@gziolo
Copy link
Member

gziolo commented Nov 28, 2020

In the ideal case, a bot would apply labels snd comment with recommendations on how to include CHANGELOG chsnges or Dev Notes 😃

We can land it after rebase, but as Riad mentioned the more items on the list the more people ignore thrm 🙁

@gziolo gziolo added the [Type] Project Management Meta-issues related to project management of Gutenberg label Nov 28, 2020
Base automatically changed from master to trunk March 1, 2021 15:43
@Mamaduka
Copy link
Member

The project now uses a simplified PR template with no checklist. See #39229.

I'm going to close this PR.

@Mamaduka Mamaduka closed this Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Project Management Meta-issues related to project management of Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants