Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

changelog for git submodules #7776

Closed
Tracked by #14138
lkwg82 opened this issue Nov 20, 2020 · 6 comments
Closed
Tracked by #14138

changelog for git submodules #7776

lkwg82 opened this issue Nov 20, 2020 · 6 comments
Labels
core:changelogs Related to changelogs/release notes fetching manager:git-submodules Git Submodules manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)

Comments

@lkwg82
Copy link

lkwg82 commented Nov 20, 2020

What would you like Renovate to be able to do?

I would like to have insights into updates offered by a new submodule version besides "hash is different" ;)

Did you already have any implementation ideas?

yes

  1. Add some parsing around git diff --submodule=diff.
  2. Add Link to compare e.g. for gitlab https://instance/namespace/project/-/compare/07dc597c73f81bcbb370678209871f990b6c94e8...0d760e9d823c33f30649e32967d2f06f5da91710

Requirements:
Have a summary of what has changed either as commit list or as diff.

@HonkingGoose HonkingGoose added the type:feature Feature (new functionality) label Nov 20, 2020
@rarkins rarkins added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others manager:git-submodules Git Submodules manager labels Nov 21, 2020
@rarkins
Copy link
Collaborator

rarkins commented Nov 21, 2020

This is not really a changelog by the usual definition. Sounds like you want a list of commits, which could also apply to other types of updates too. We stopped including them in repos at one point because it can introduce a huge amount of noise in some cases

@lkwg82
Copy link
Author

lkwg82 commented Nov 23, 2020

@rarkins added a requirement.

@rarkins
Copy link
Collaborator

rarkins commented Nov 23, 2020

I think a diff compare link would definitely be good

@lkwg82
Copy link
Author

lkwg82 commented Nov 23, 2020

I tried already to build the link with the template variables, however the link was only given as markdown. Unwrapping markdown and adding parameters was not successfull :/.

@rarkins rarkins added the status:requirements Full requirements are not yet known, so implementation should not be started label Jan 12, 2021
@viceice viceice added the core:changelogs Related to changelogs/release notes fetching label Feb 10, 2022
@goodtune
Copy link

It would be nice to add a comment on the submodule path as markdown with inline code flagged as a diff -- for example, when this is the diff

diff --git a/renovate b/renovate
index 9e28ef3..70084a6 160000
--- a/renovate
+++ b/renovate
@@ -1 +1 @@
-Subproject commit 9e28ef32752536501af3fbff8a63ccbe5343591c
+Subproject commit 70084a61ef43a8d927b30b99577dd26ae40950d3

We could comment by using the output of git diff --submodule=diff like so:

Submodule renovate 9e28ef3..70084a6 (rewind):
diff --git a/renovate/lib/modules/datasource/datasource.ts b/renovate/lib/modules/datasource/datasource.ts
index 3d7fa807a..eb9ffa1ce 100644
--- a/renovate/lib/modules/datasource/datasource.ts
+++ b/renovate/lib/modules/datasource/datasource.ts
@@ -19,7 +19,7 @@ export abstract class Datasource implements DatasourceApi {
 
   defaultConfig: Record<string, unknown> | undefined;
 
-  defaultRegistryUrls?: string[] | (() => string[]);
+  defaultRegistryUrls: string[] | undefined;
 
   defaultVersioning: string | undefined;
 
diff --git a/renovate/lib/modules/datasource/index.spec.ts b/renovate/lib/modules/datasource/index.spec.ts
index ace1cba91..c2054c683 100644
--- a/renovate/lib/modules/datasource/index.spec.ts
+++ b/renovate/lib/modules/datasource/index.spec.ts
@@ -43,47 +43,6 @@ class DummyDatasource extends Datasource {
   }
 }
 
-class DummyDatasource2 extends Datasource {
-  override defaultRegistryUrls = function () {
-    return ['https://reg1.com'];
-  };
-
-  constructor(private registriesMock: RegistriesMock = defaultRegistriesMock) {
-    super(datasource);
-  }
-
-  override getReleases({
-    registryUrl,
-  }: GetReleasesConfig): Promise<ReleaseResult | null> {
-    const fn = this.registriesMock[registryUrl];
-    if (typeof fn === 'function') {
-      return Promise.resolve(fn());
-    }
-    return Promise.resolve(fn ?? null);
-  }
-}
-
-class DummyDatasource3 extends Datasource {
-  override customRegistrySupport = false;
-  override defaultRegistryUrls = function () {
-    return ['https://reg1.com'];
-  };
-
-  constructor(private registriesMock: RegistriesMock = defaultRegistriesMock) {
-    super(datasource);
-  }
-
-  override getReleases({
-    registryUrl,
-  }: GetReleasesConfig): Promise<ReleaseResult | null> {
-    const fn = this.registriesMock[registryUrl];
-    if (typeof fn === 'function') {
-      return Promise.resolve(fn());
-    }
-    return Promise.resolve(fn ?? null);
-  }
-}
-
 jest.mock('./metadata-manual', () => ({
   manualChangelogUrls: {
     dummy: {
@@ -253,30 +212,6 @@ describe('modules/datasource/index', () => {
       expect(res).toMatchObject({ releases: [{ version: '0.0.1' }] });
     });
 
-    it('defaultRegistryUrls function works', async () => {
-      datasources.set(datasource, new DummyDatasource2());
-      const res = await getPkgReleases({
-        datasource,
-        depName,
-      });
-      expect(res).toMatchObject({
-        releases: [{ version: '1.2.3' }],
-        registryUrl: 'https://reg1.com',
-      });
-    });
-
-    it('defaultRegistryUrls function with customRegistrySupport works', async () => {
-      datasources.set(datasource, new DummyDatasource3());
-      const res = await getPkgReleases({
-        datasource,
-        depName,
-      });
-      expect(res).toMatchObject({
-        releases: [{ version: '1.2.3' }],
-        registryUrl: 'https://reg1.com',
-      });
-    });
-
     it('applies extractVersion', async () => {
       const registries: RegistriesMock = {
         'https://reg1.com': {
diff --git a/renovate/lib/modules/datasource/index.ts b/renovate/lib/modules/datasource/index.ts
index ece88626a..b55b1c2e1 100644
--- a/renovate/lib/modules/datasource/index.ts
+++ b/renovate/lib/modules/datasource/index.ts
@@ -209,9 +209,7 @@ function resolveRegistryUrls(
         'Custom registries are not allowed for this datasource and will be ignored'
       );
     }
-    return is.function_(datasource.defaultRegistryUrls)
-      ? datasource.defaultRegistryUrls()
-      : datasource.defaultRegistryUrls ?? [];
+    return datasource.defaultRegistryUrls ?? [];
   }
   const customUrls = registryUrls?.filter(Boolean);
   let resolvedUrls: string[] = [];
@@ -220,9 +218,6 @@ function resolveRegistryUrls(
   } else if (is.nonEmptyArray(defaultRegistryUrls)) {
     resolvedUrls = [...defaultRegistryUrls];
     resolvedUrls.concat(additionalRegistryUrls ?? []);
-  } else if (is.function_(datasource.defaultRegistryUrls)) {
-    resolvedUrls = [...datasource.defaultRegistryUrls()];
-    resolvedUrls.concat(additionalRegistryUrls ?? []);
   } else if (is.nonEmptyArray(datasource.defaultRegistryUrls)) {
     resolvedUrls = [...datasource.defaultRegistryUrls];
     resolvedUrls.concat(additionalRegistryUrls ?? []);
diff --git a/renovate/lib/modules/datasource/types.ts b/renovate/lib/modules/datasource/types.ts
index 867c8829f..be5e106ec 100644
--- a/renovate/lib/modules/datasource/types.ts
+++ b/renovate/lib/modules/datasource/types.ts
@@ -76,7 +76,7 @@ export interface DatasourceApi extends ModuleApi {
   id: string;
   getDigest?(config: DigestConfig, newValue?: string): Promise<string | null>;
   getReleases(config: GetReleasesConfig): Promise<ReleaseResult | null>;
-  defaultRegistryUrls?: string[] | (() => string[]);
+  defaultRegistryUrls?: string[];
   defaultVersioning?: string;
   defaultConfig?: Record<string, unknown>;

This would give reviewers of the pull request the context of the change between the two revisions.

We could also do the commit log with git diff --submodule=log as below.

Submodule renovate 9e28ef3..70084a6 (rewind):
  < refactor: redefine defaultRegistryUrls (#15856)

This is all from a very toy repository I made locally, added this project as a submodule, and rewound it by a single commit.

@agners
Copy link

agners commented May 1, 2023

A changelog would be very much appreciated here as well!

I am using git submodule summary currenlty. There is a --summary-limit option which limits the summary to a certain amount of commits.

@renovatebot renovatebot locked and limited conversation to collaborators Oct 1, 2023
@rarkins rarkins converted this issue into discussion #24903 Oct 1, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
core:changelogs Related to changelogs/release notes fetching manager:git-submodules Git Submodules manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

6 participants