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

feat(repo/changelogs): allow user configuration of source url #16873

Conversation

Gabriel-Ladzaretti
Copy link
Collaborator

@Gabriel-Ladzaretti Gabriel-Ladzaretti commented Jul 31, 2022

Changes

Introduces a new config option named overwriteSourceUrl which is valid within the PackageRules object only (not enforced, a warning message will be logged).

When set, this url will be used as the source of changelogs retrieval.
This url is now passed a function parameter to both getChangeLogJSON functions.

Cache namespace changelog-github/github-release which is used for caching the Compare Source links. which is originally structured as - manager:depname:prevVer:nextVer ,will now also contain the repository/pathname to prevent conflicts + use datasource instead of manager as part of the key., is now sourceUrl:depname:prevVer:nextVer.

Note: sourceUrl is restored to its original value after changelog retrieval, this is due the fact that it is part of the repository cache. and if im not mistaken changelogs has no meaning in the context of repository cache but the original datasource fetched sourceUrl does.

Context

closes #16011

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@Gabriel-Ladzaretti Gabriel-Ladzaretti marked this pull request as ready for review July 31, 2022 11:41
@rarkins
Copy link
Collaborator

rarkins commented Jul 31, 2022

Shouldn't the cache namespace be related to datasource and not manager?

@Gabriel-Ladzaretti
Copy link
Collaborator Author

Shouldn't the cache namespace be related to datasource and not manager?

probably, as the source url is being set in the datasource stage.
should i change it in this PR?

@rarkins
Copy link
Collaborator

rarkins commented Jul 31, 2022

Yes I think if you're making changes then now is a good time. Let's consider if other fields need changing too

@Gabriel-Ladzaretti
Copy link
Collaborator Author

Gabriel-Ladzaretti commented Jul 31, 2022

Yes I think if you're making changes then now is a good time. Let's consider if other fields need changing too

I think this might be deeper than just changing to datasource as it's type is string | undefiend
while manager is strictly a string as per

I mean we can asset not null (! operator) but im not sure its entirely true at runtime

edit: on second thought, this might not be a problem

const versioning = config.versioning!;
const currentVersion = config.currentVersion!;
const newVersion = config.newVersion!;
const sourceUrl = config.sourceUrl!;
const sourceDirectory = config.sourceDirectory!;
const depName = config.depName!;
const manager = config.manager;

@Gabriel-Ladzaretti
Copy link
Collaborator Author

Gabriel-Ladzaretti commented Jul 31, 2022

other namespaces and cache keys

GitHub

mem cache

const cacheKey = `getTags-${endpoint}-${repository}`;

package cache - key modified: repository added, manager change to datasource

const cacheNamespace = 'changelog-github-release';
function getCacheKey(prev: string, next: string): string {
return `${manager}:${depName}:${prev}:${next}`;
}

GitLab

memCache

const cacheKey = `getTags-${endpoint}-${versionScheme}-${repository}`;

package cache - key modified: pathname added, manager change to datasource

const cacheNamespace = 'changelog-gitlab-release';

function getCacheKey(prev: string, next: string): string {
return `${manager}:${depName}:${prev}:${next}`;
}

Release notes

packageCache

const cacheNamespace = `changelog-${input.project.type}-notes@v2`;
function getCacheKey(version: string): string {
return `${repository}:${
sourceDirectory ? `${sourceDirectory}:` : ''
}${version}`;
}

mem cache

const cacheKey = `getReleaseList-${project.apiBaseUrl!}-${
project.repository
}`;

const cacheKey = `getReleaseNotesMdFile@v2-${project.repository}${
project.sourceDirectory ? `-${project.sourceDirectory}` : ''
}-${project.apiBaseUrl!}`;

package cache

const cacheNamespace = `changelog-${input.project.type}-notes@v2`;
function getCacheKey(version: string): string {
return `${repository}:${
sourceDirectory ? `${sourceDirectory}:` : ''
}${version}`;
}

@Gabriel-Ladzaretti
Copy link
Collaborator Author

now using datasource as part of the relevant cache keys.
commit a760e03

docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
lib/workers/repository/update/pr/changelog/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

if it's only used for changelogs, shouldn't we name it customChangelogUrl ? and use that if set in changelogs?

@Gabriel-Ladzaretti
Copy link
Collaborator Author

if it's only used for changelogs, shouldn't we name it customChangelogUrl ? and use that if set in changelogs?

you mean to change all sourceUrl -> customChangelogUrl ?? sourceUrl!, so basically customChangelogUrl have precedence over sourceUrl

i guess it depends on how many places we are using sourceUrl directly,

@viceice
Copy link
Member

viceice commented Aug 11, 2022

yes, but only in changelogs.

@viceice
Copy link
Member

viceice commented Aug 11, 2022

maybe also needs customChangelogsDirectory

@viceice
Copy link
Member

viceice commented Aug 11, 2022

those should then passt to all functions in changelogs, so they are evaluated once, like platform detection

@Gabriel-Ladzaretti
Copy link
Collaborator Author

Gabriel-Ladzaretti commented Aug 11, 2022

those should then passt to all functions in changelogs, so they are evaluated once, like platform detection

im not that familiar with this flow, do you mean directory in terms of caching? if so, in current state all changelogs even overridden ones are getting cached, the keying was changed in a defensive way such that if a user overwrites the sourceUrl of a well known dep its key will reflect that.

thats why in the POC i used the same dependency, one repo uses a custom url and the next uses the default one and there was no interference between them

@rarkins
Copy link
Collaborator

rarkins commented Aug 11, 2022

maybe also needs customChangelogsDirectory

It would be nice if we can overcome the barriers to reusing all our existing field names, rather than defining each one again

@Gabriel-Ladzaretti Gabriel-Ladzaretti marked this pull request as draft August 18, 2022 06:55
@Gabriel-Ladzaretti Gabriel-Ladzaretti marked this pull request as ready for review August 18, 2022 08:25
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

see other comments

@Gabriel-Ladzaretti Gabriel-Ladzaretti force-pushed the 16011-Allow_user_configuration_of_sourceUrl branch from c60dbb4 to 2397fed Compare August 22, 2022 14:22
@rarkins rarkins enabled auto-merge (squash) August 22, 2022 19:51
@rarkins rarkins merged commit a54cc47 into renovatebot:main Aug 22, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.172.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow user configuration of sourceUrl
4 participants