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 workflow to check for expected @wordpress/components CHANGELOG updates #49443

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

chad1008
Copy link
Contributor

@chad1008 chad1008 commented Mar 29, 2023

What?

Adds an automated check to PRs editing the @wordpress/components package to ensure the CHANGELOG has been updated.

Why?

This is an easy step to miss, and a common part of reviews, so it makes sense to automate it.

How?

The new check will only be run on PRs that change files in the packages/components/** directory, but not if all of those files are stories or unit tests, as those changes don't need to be entered in the changelog.

If any files outside of tests and stories are modified, a git diff is run to confirm that /packages/components/CHANGELOG.md is included in the files changed by the PR. If it isn't, the check fails with an error message asking the author to add an entry.

Testing Instructions

  1. After checking out this PR, create a new branch off of it. Create a PR of your own from that branch.
  2. Modify a story file for any component. Commit that change to your test branch and push, updating your PR.
  3. Wait for the checks to run and confirm that Verify @wordpress/components CHANGELOG update was NOT run.
  4. Modify a unit test for any component. Commit that change to your test branch and push, updating your PR again.
  5. Wait again for the checks and confirm that Verify @wordpress/components CHANGELOG update still was NOT run.
  6. Modify a component file (any component, anything that isn't a unit test or story file). Commit and push.
  7. Wait for the checks. Confirm that this time Verify @wordpress/components CHANGELOG update IS run.
  8. Confirm that the check fails, because you didn't update the changelog. When clicking details on the check, you should see output asking you to add a CHANGELOG entry.
  9. Update the CHANGELOG file. The check isn't picky about what you do, it only looks for the file to have been changed. Commit and push one last time.
  10. Wait once more for the checks, confirm that this time Verify @wordpress/components CHANGELOG update runs and passes.

@chad1008 chad1008 closed this Mar 29, 2023
@chad1008 chad1008 deleted the components-changelog-check branch March 29, 2023 14:57
@chad1008 chad1008 restored the components-changelog-check branch March 29, 2023 15:28
@chad1008 chad1008 reopened this Mar 29, 2023
@github-actions
Copy link

github-actions bot commented Mar 29, 2023

Flaky tests detected in 4c8bea2.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4620681022
📝 Reported issues:

@chad1008 chad1008 force-pushed the components-changelog-check branch 11 times, most recently from 3b9dd59 to 514cb9a Compare April 4, 2023 16:54
@chad1008 chad1008 marked this pull request as ready for review April 4, 2023 16:56
@chad1008 chad1008 self-assigned this Apr 4, 2023
@chad1008 chad1008 added [Package] Components /packages/components GitHub Actions Pull requests that update GitHub Actions code labels Apr 4, 2023
- name: Checkout code
uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
- name: Fetch origin/trunk
run: git fetch --quiet origin trunk
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if I'm reading this correctly, but it looks like the search for changes to the CHANGELOG file is done against the latest trunk (ie. from the origin remote). I wonder what happens if the CHANGELOG file is changed on trunk?

In my mental model, we should check for differences against the commit that the current PR is based off (something similar to what described in this article), and not against latest origin / trunk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, and thanks for the example, it helped a lot.

I've reworked the PR so it should now check against the PR's base commit. While changing this, I also realized I didn't like that we were fetching all of origin/trunk when we really only need to go back as far as the base commit, so I've made the fetch depth dynamic to only pull what the given PR needs.

@chad1008 chad1008 requested a review from ajitbohra as a code owner April 5, 2023 11:23
@chad1008 chad1008 force-pushed the components-changelog-check branch 2 times, most recently from 28e2390 to d338282 Compare April 5, 2023 13:20
@ciampo ciampo mentioned this pull request Apr 5, 2023
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

Tested in #49619, works as per testing instructions 🚀

chad1008 and others added 4 commits April 5, 2023 12:26
use workflow `paths` filter instead of scripted logic

fix test folder exclusion

add git fetch origin

uncomment the logic because im a dummy

fetch origin/trunk only

suppress git fetch output

update naming and step structure
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
@chad1008 chad1008 enabled auto-merge (squash) April 5, 2023 16:28
@chad1008 chad1008 merged commit 0cb1c2e into trunk Apr 5, 2023
@chad1008 chad1008 deleted the components-changelog-check branch April 5, 2023 16:58
@github-actions github-actions bot added this to the Gutenberg 15.6 milestone Apr 5, 2023
@geriux
Copy link
Member

geriux commented Apr 10, 2023

Hey there 👋 I'm having issues in this PR where the Verify @wordpress/components CHANGELOG update / Check CHANGELOG diff check fails. @chad1008 do you have any ideas why is it failing? Thanks!

@geriux
Copy link
Member

geriux commented Apr 10, 2023

I see it is because I'm making changes to some files within the components folder but it's for native files, I think we should exclude this check for .native files 🤔

@chad1008
Copy link
Contributor Author

Thanks for pointing that out @geriux. Proposed a change to the check in #49688 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Actions Pull requests that update GitHub Actions code [Package] Components /packages/components [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants