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
changeset completeness check #1494
Conversation
|
throw new Error( | ||
`The following packages have changes but are not included in any changeset:${EOL}${EOL}${packagesMissingChangesets.join( | ||
EOL | ||
)}${EOL}${EOL}Add a changeset using 'npx changeset add' or 'npx changeset add --empty' if this is a non-functional change.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think message now should clarify that --empty
is allowed if only private packages are changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things like adding tests or linting a public package could also have an empty changeset. Although maybe those cases are small enough that it's not worth mentioning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
We could perhaps have some predicate function in the algorithm that narrows down this classification.
something like
const shouldHaveNonEmptyChangeSet: () : 'yes'|'no'|'maybe' => {
if (API.md changed) {
return yes
}
if (Only Test Changed) {
return no
}
return maybe;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave this as a future optimization if needed. For now, I just removed the empty changeset suggestion. Having an unnecessary version bump is much better than a missing version bump.
Problem
Our current changeset checks assert that PR that touch public packages have a changeset, but it does not check that all packages with changes are actually included in a changeset. This puts an unnecessary manual burden on the PR author and reviewer to ensure that all modified packages are included in a changeset.
Issue number, if available:
Changes
Add a PR check to validate that any modified package is included in a changeset
Corresponding docs PR, if applicable:
Validation
Running some experiments on this PR
Checklist
run-e2e
label set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.