fix(package-rules): make fetchChangeLogs work at update level - #42368
justfalter wants to merge 10 commits into
Conversation
jamietanna
left a comment
There was a problem hiding this comment.
We should also mention this in https://docs.renovatebot.com/key-concepts/changelogs/
| description: | ||
| 'Set to true to disable fetching of changelogs for matching packages.', | ||
| type: 'boolean', | ||
| stage: 'pr', |
There was a problem hiding this comment.
This might also be branch-level - if fetchChangeLogs=branch
|
I think this seems reasonable - I'd like |
viceice
left a comment
There was a problem hiding this comment.
I would prefer to allow using the existing fetch changelog option at package level instead of branch level.
|
|
||
| For more details on supported syntax see Renovate's [string pattern matching documentation](./string-pattern-matching.md). | ||
|
|
||
| ### packageRules.disableChangeLog |
There was a problem hiding this comment.
why not using the existing fetch changelog option 🤔
There was a problem hiding this comment.
I chose packageRules.disableChangeLog over packageRules.fetchChangeLogs only to keep things simple, but I am open to change.
If we were add packageRules.fetchChangeLogs, I suppose that the package rule would simply override the top-level. This should still allow for what I am looking to accomplish (disabling changelogs for specific packages), while also affording greater flexibility.
This would mean moving more decision-making into embedChangelog, which I think is reasonable.
4bc8b5b to
bccd287
Compare
Adds `packageRules.disableChangeLog`, a means for disabling changelog fetching for matching packages.
- replace `packageRules.disableChangeLog` with a `packageRules.fetchChangeLogs`. - Update docs and tests.
bccd287 to
c56d1a5
Compare
| header !== 'managerFilePatterns' && header !== 'enabled', | ||
| header !== 'managerFilePatterns' && | ||
| header !== 'enabled' && | ||
| header !== 'fetchChangeLogs', |
There was a problem hiding this comment.
This was required in order to get documentation tests to pass as the configuration.md now contains a fetchChangeLogs and a packageRules.fetchChangeLogs header.
There was a problem hiding this comment.
remote packageRules.fetchChangeLogs and add a note to fetchChangeLogs. We've other options which are using same (eg enabled)
|
I have refactored things into Here's my renovate config: https://github.com/justfalter/renovate-test-repo/blob/main/.github/renovate.json5 Here's an action run w/ the above config (note: only takes 1m17s w/ |
c763ac2 to
afdbf0e
Compare
| allowedValues: ['off', 'branch', 'pr'], | ||
| default: 'pr', | ||
| cli: false, | ||
| parents: ['packageRules', '.'], |
There was a problem hiding this comment.
| parents: ['packageRules', '.'], | |
| parents: ['.', 'packageRules'], |
global option should still be first. @jamietanna WDYT?
| header !== 'managerFilePatterns' && header !== 'enabled', | ||
| header !== 'managerFilePatterns' && | ||
| header !== 'enabled' && | ||
| header !== 'fetchChangeLogs', |
There was a problem hiding this comment.
remote packageRules.fetchChangeLogs and add a note to fetchChangeLogs. We've other options which are using same (eg enabled)
| >; | ||
|
|
||
| export interface EmbedChangelogsOptions { | ||
| branches: BranchUpgradeConfig[]; |
There was a problem hiding this comment.
wrong name
| branches: BranchUpgradeConfig[]; | |
| upgrades: BranchUpgradeConfig[]; |
|
|
||
| // The Extend utility type just ensures that U is a subset of T, making sure that we get a typescript error should | ||
| // 'branch' or 'pr' ever be removed from the FetchChangeLogOptions union. | ||
| type Extends<T, U extends T> = U; |
| } | ||
|
|
||
| export async function embedChangelogs({ | ||
| branches, |
There was a problem hiding this comment.
| branches, | |
| upgrades, |
| fetchChangeLogs, | ||
| }: EmbedChangelogsOptions): Promise<void> { | ||
| // Filter down to branch upgrades that match the stage and fetchChangeLogs configuration. | ||
| const upgrades = branches.filter( |
There was a problem hiding this comment.
| const upgrades = branches.filter( | |
| const filteredUpgrades = upgrades.filter( |
| resolveFetchChangeLogs(fetchChangeLogs, upgrade.fetchChangeLogs) === | ||
| stage, | ||
| ); | ||
| await p.map(upgrades, embedChangelog, { concurrency: 10 }); |
There was a problem hiding this comment.
| await p.map(upgrades, embedChangelog, { concurrency: 10 }); | |
| await p.map(filteredUpgrades, embedChangelog, { concurrency: 10 }); |
| // Merges the top-level fetchChangeLogs value with the upgrade's fetchChangeLogs value (prioritizing the latter, if defined). | ||
| function resolveFetchChangeLogs( | ||
| fetchChangeLogs?: FetchChangeLogsOptions, | ||
| upgradeFetchChangeLogs?: FetchChangeLogsOptions, |
There was a problem hiding this comment.
i think this is always set on upgrades object 🤔
please create another reproduction:
- set
fetchChangeLogs=offglobally - set
fetchChangeLogsPrfor a single dep - group deps to single PR
- show us the link 😁
fetchChangeLogs option
fetchChangeLogs option|
I've spent the morning doing additional testing outside of this pull-request, and it looks like Renovate already supports specifying I'll be honest, I am really not certain how I had gotten this far without validating that it could not already be set in this way. I had been working off the documentation, and I thought I had attempted a This is all to say that this pull-request is not needed. |
|
I'm re-opening this pull request after I found that the |
- Revert chanages that added `fetchChangeLogs` to types --- they were already present. - Remove docs for `packageRules.fetchChangeLogs` and put it under `fetchChangeLogs`. - Revert changes to options - they are not nescessary, as is the case for most other options that can be overridden at the packageRules level.
|
First update: 9873069 and simplifying documentation. |
- top-level: only choose options that are implicitly root level (no parents) or explicitly have root ('.') as parent.
- sub-options: only choose options that have parents and none of them are root-level ('.').
This means that we no longer have to exclude for `enabled`
Ensures that changelog fetching respects each dep update's specific `fetchChangeLogs` value. Given the following renovate configuration: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/blob/fb9e070d0f9a4d6fc742c4a22edee2c9b055fece/.github/renovate.json5#L42-L61 My expectation is that Renovate would never fetch changelogs for `lodash`. However, when grouped together with `chalk` and `tar` under `group of unrelated packages`, we find that renovate fetches changelogs all packages, including `lodash`. Before this change (`lodash` logs fetched): - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24480887343/job/71544876893 - (wrong) lodash, tar, and chalk all have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#9 After this change, the PR created by renovate only contains changelogs for `chalk` and `tar`, but not `lodash`: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24481738609/job/71547632550 - (expected) lodash does not have changelogs, but tar and chalk have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#10 ---- We can further explore the effectiveness of the change with a more elaborate configuration, where the top-level has `fetchChangeLogs: off`: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/blob/0ea69c18b9171e38c56440c762faa68109d3d0d5/.github/renovate.json5#L41-L95 Before the change: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24482213234/job/71549162174 - (wrong) lodash, tar, and chalk all have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#15 - (expected) typescript (inherited `off`) does not have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#17 - (expected) grunt (overridden `pr`) has changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#13 After the change:: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24482561172/job/71550255278 - (expected) lodash does not have changelogs, but tar and chalk have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#22 - (expected) typescript (inherited `off`) does not have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#24 - (expected) grunt (overridden `pr`) has changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#20 Portions of this PR previously appeared in renovatebot#42368
|
Closing this pull request in favor of #42671 |
…enovatebot#42671) * fix(package-rules): respect each dep update's fetchChangeLogs value Ensures that changelog fetching respects each dep update's specific `fetchChangeLogs` value. Given the following renovate configuration: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/blob/fb9e070d0f9a4d6fc742c4a22edee2c9b055fece/.github/renovate.json5#L42-L61 My expectation is that Renovate would never fetch changelogs for `lodash`. However, when grouped together with `chalk` and `tar` under `group of unrelated packages`, we find that renovate fetches changelogs all packages, including `lodash`. Before this change (`lodash` logs fetched): - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24480887343/job/71544876893 - (wrong) lodash, tar, and chalk all have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#9 After this change, the PR created by renovate only contains changelogs for `chalk` and `tar`, but not `lodash`: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24481738609/job/71547632550 - (expected) lodash does not have changelogs, but tar and chalk have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#10 ---- We can further explore the effectiveness of the change with a more elaborate configuration, where the top-level has `fetchChangeLogs: off`: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/blob/0ea69c18b9171e38c56440c762faa68109d3d0d5/.github/renovate.json5#L41-L95 Before the change: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24482213234/job/71549162174 - (wrong) lodash, tar, and chalk all have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#15 - (expected) typescript (inherited `off`) does not have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#17 - (expected) grunt (overridden `pr`) has changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#13 After the change:: - Run: https://github.com/justfalter/renovate-test-packagerules-fetchchangelogs/actions/runs/24482561172/job/71550255278 - (expected) lodash does not have changelogs, but tar and chalk have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#22 - (expected) typescript (inherited `off`) does not have changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#24 - (expected) grunt (overridden `pr`) has changelogs: justfalter/renovate-test-packagerules-fetchchangelogs#20 Portions of this PR previously appeared in renovatebot#42368 * pre feedback: just use fetchChangeLogs from upgrade * revert documentation.spec.ts changes They've already been integrated with renovatebot#42846
Changes
Adds
packageRules.fetchChangeLogs, a means for overriding changelog fetching for matching packages.The goal of this change is to provide package-level control over the fetching of changelogs. It allows users to weigh the tradeoff between changelogs and runtime for things like large monorepos (ex: https://github.com/aws/aws-sdk-go-v2), or perhaps to simply disable changelogs for problematic hosts.
My personal motivation, as outlined in a discussion thread I started the other day, centers around my need to mitigate the impact that fetching changelogs on a GitHub Enterprise instance for monorepos with a large number of tags, as well as eliminate redundancy (I have internal tooling for handling changelogs).
The
packageRules.disableChangeLogoption will allow me to add a shared preset to my organization for disabling changelog generation for packages sourced from my internal monorepo via the simple:Relevant renovate runs in GitHub actions:
disableChangeLog: 14m29s in total (most fetching changelogs for aws-sdk-go-v2) - created PR.Run w/
disableChangeLog: trueforaws-sdk-go-v2andaws-sdk-js-v3: 1m54s - changelogs were produced for all updates (example), except the targeted aws-sdk packages (PR).Context
Please select one of the following:
Please see #42358
AI assistance disclosure
Did you use AI tools to create any part of this pull request?
Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.
Used Claude to understand the codebase, tracing connections between configuration and affecting change, as well as to help generate the tests (which I reviewed).
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via:
The public repository: https://github.com/justfalter/renovate-test-repo