-
Notifications
You must be signed in to change notification settings - Fork 14
Add Support for Retaining next Release Information in Changelogs
#185
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
=======================================
Coverage 73.36% 73.36%
=======================================
Files 85 85
Lines 12932 12932
Branches 1213 1213
=======================================
Hits 9487 9487
Misses 3424 3424
Partials 21 21 ☔ View full report in Codecov by Sentry. |
scripts/next-changelogs.js
Outdated
| const [latest, second] = getLatestReleaseTags(); | ||
| if (dryRun) { | ||
| auto.logger.log.info( | ||
| `Dry run: making changelog from last release: ${latestRelease}`, |
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.
Is latestRelease defined somewhere? Or should this be latest instead?
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.
Ah, good catch, let me fix that.
|
|
||
| apply(auto) { | ||
| auto.hooks.next.tapPromise(this.name, async ({ dryRun }) => { | ||
| const [latest, second] = getLatestReleaseTags(); |
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.
Can any edge case cause getLatestReleaseTags() to return less than two tags? If so, this will fail when destructuring..
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.
In theory yes but since this repo already has multiple releases we should always have a to <-> from
| } | ||
| } | ||
|
|
||
| module.exports = NextChangelogsPlugin; |
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.
Should we use named export instead?
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.
In general our convention is named exports but for auto plugins they have to be default exports in order to be applied
Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ARelease Notes
Keep information about
nextrelease in changelog