Skip to content

Add way to verify each change has associated PRs#222

Merged
mcmire merged 1 commit intomainfrom
ensure-pr-links
Mar 5, 2025
Merged

Add way to verify each change has associated PRs#222
mcmire merged 1 commit intomainfrom
ensure-pr-links

Conversation

@mcmire
Copy link
Copy Markdown
Contributor

@mcmire mcmire commented Feb 25, 2025 •
edited
Loading

This commit adds a new option to the validate command, --pr-links, which will cause an error to be thrown if:

  • a changelog entry does not have one or more links to pull requests after it
  • a changelog entry does have PR links present, but they do not point to the project's repo
  • a changelog entry does have PR links present, but they are not positioned at the very end of the line

The ensureValidPrLinksPresent option has also been added to validateChangelog.

If this option is provided, then parseChangelog is instructed to look for and extract pull request numbers from changelog entries. The list of numbers will then be checked for in the validation step. It is also used to reconstruct pull request links when the changelog is stringified.

Note that because this commit changes what parseChangelog returns, this is a breaking change.

Closes #150.

Changelog

Added

  • Add a way to ensure that each changelog entry has a well-formatted list of PR links at the end of the line (or throws if this is not the case).
    • Add --pr-links / --prLinks to the validate command
    • Add ensureValidPrLinksPresent to validateChangelog
  • Add shouldExtractPrLinks to parseChangelog which, if true, will attempt to look for and extract PR links/numbers from each changelog entry.
  • Add optional option prNumbers to Changelog.addChange, which will be combined with the description when the changelog is stringified.

Changed

  • BREAKING: Changelog.getReleaseChanges and Changelog.getUnreleasedChanges now return an array of objects ({ description, prNumbers }) rather than array of strings.

Manual testing

  • Run yarn build
  • Run yarn run changelog validate --pr-links. This will validate this package's changelog, checking for PR links.
  • See that validation currently fails because one of the entries in 1.0.0 is invalid. Correct this by placing the period before the PR link.
  • Run yarn run changelog validate --pr-links again.
  • See that validation fails again because of the entries in 2.3.0 has duplicated links. Remove the duplicate links.
  • Run yarn run changelog validate --pr-links again.
  • See no output.

@mcmire mcmire requested a review from a team as a code owner February 25, 2025 19:29
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 25, 2025
View reviewed changes
src/changelog.ts
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
release[category]!.push(description);
}
release[category] = releaseCategory;

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
library input
Loading
.

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to ensure that the version parameter cannot be a special property name like __proto__, constructor, or prototype. This can be done by adding a validation check for the version parameter before it is used to index into the this.#changes object.

  1. Add a validation check for the version parameter to ensure it is not a special property name.
  2. If the version parameter is invalid, throw an error to prevent further execution.
Suggested changeset 1
src/changelog.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/changelog.ts b/src/changelog.ts
--- a/src/changelog.ts
+++ b/src/changelog.ts
@@ -516,2 +516,4 @@
       throw new Error('Description required');
+    } else if (version !== undefined && (version === '__proto__' || version === 'constructor' || version === 'prototype')) {
+      throw new Error(`Invalid version: '${version}'`);
     } else if (version !== undefined && !this.#changes[version]) {
EOF
@@ -516,2 +516,4 @@
throw new Error('Description required');
} else if (version !== undefined && (version === '__proto__' || version === 'constructor' || version === 'prototype')) {
throw new Error(`Invalid version: '${version}'`);
} else if (version !== undefined && !this.#changes[version]) {
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Copy Markdown
Contributor Author

@mcmire mcmire Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I will fix this problem in a new PR since it already exists.

@mcmire
Add way to verify each change has associated PRs
803ae97 
This commit adds a new option to the `validate` command, `--pr-links`,
which will cause an error to be thrown if:

- a changelog entry does not have one or more links to pull requests
  after it
- a changelog entry does have PR links present, but they do not point to
  the project's repo
- a changelog entry does have PR links present, but they are not
  positioned at the very end of the line

The `ensureValidPrLinksPresent` option has also been added to
`validateChangelog`.

If this option is provided, then `parseChangelog` is instructed to look
for and extract pull request numbers from changelog entries. The list of
numbers will then be checked for in the validation step. It is also used
to reconstruct pull request links when the changelog is stringified.

Note that because this commit changes what `parseChangelog` returns,
this is a breaking change.
@mcmire mcmire mentioned this pull request Feb 25, 2025
Closed
cryptodev-2s
cryptodev-2s approved these changes Mar 5, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM!

@mcmire mcmire merged commit b7c5c0e into main Mar 5, 2025
15 checks passed
@mcmire mcmire deleted the ensure-pr-links branch March 5, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cryptodev-2s cryptodev-2s cryptodev-2s approved these changes

Assignees

No one assigned

Labels

team-wallet-framework

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a way to verify that each changelog entry has an associated PR

3 participants

@mcmire @github-advanced-security @cryptodev-2s