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

[GitHub] enable platformCommit when we detect an app token #29115

Open
viceice opened this issue May 16, 2024 · 6 comments · Fixed by #29156
Open

[GitHub] enable platformCommit when we detect an app token #29115

viceice opened this issue May 16, 2024 · 6 comments · Fixed by #29156
Assignees
Labels
breaking Breaking change, requires major version bump platform:github GitHub Platform priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)

Comments

@viceice
Copy link
Member

viceice commented May 16, 2024

Describe the proposed change(s).

Change platformCommit to enum like with values enabled, auto and disabled.
Default to auto.
Set to enabled when it's auto and we detect an github app token.
i think they start with ghs_.

export function isGithubServerToServerToken(token: string): boolean {
return token.startsWith('ghs_');
}

@viceice viceice added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others platform:github GitHub Platform labels May 16, 2024
@viceice viceice changed the title enable platformCommit when we detect a app token and not disabled [GitHub] enable platformCommit when we detect a app token and not disabled May 16, 2024
@viceice viceice changed the title [GitHub] enable platformCommit when we detect a app token and not disabled [GitHub] enable platformCommit when we detect an app token May 16, 2024
@rarkins rarkins added the breaking Breaking change, requires major version bump label May 16, 2024
@rarkins
Copy link
Collaborator

rarkins commented May 16, 2024

Added breaking tag, let's do it for v38

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented May 19, 2024

Is it necessary to do it via token or can we just enabled it when platform is GIthubApp?

When we perform initPlatform we use the token to check whether the platform is a GithubApp and store this information locally in the platformConfig object using isGhApp boolean field. We can reuse this value.

token = token.replace(/^ghs_/, 'x-access-token:ghs_');
platformConfig.isGHApp = token.startsWith('x-access-token:');

@rarkins
Copy link
Collaborator

rarkins commented May 19, 2024

So will the worker keep platformCommit=auto for the whole run? Where would the logic lie?

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented May 19, 2024

We only use this option for Github so we can keep the logic in platform/github/scm, so that might not be an issue even if it's not ideal.

From

return commitConfig.platformCommit
? commitFiles(commitConfig)
: git.commitFiles(commitConfig);
}

To

export class GithubScm extends DefaultGitScm {
  override commitAndPush(
    commitConfig: CommitFilesConfig,
  ): Promise<LongCommitSha | null> {
    let platformCommit = commitConfig.platformCommit;
    if (platformCommit === 'auto' && isGHApp()) {
      platformCommit = 'enabled';
    }

    return commitConfig.platformCommit === 'enabled'
      ? commitFiles(commitConfig)
      : git.commitFiles(commitConfig);
  }
}

The isGhApp fn will reside in platform/github/index

export function isGHApp(): boolean {
  return !!platformConfig.isGHApp;
}

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented May 19, 2024

We can also move this logic to the renovateRepository fn.
Around here:

if (config.semanticCommits === 'auto') {
config.semanticCommits = await detectSemanticCommits();
}

This way platformCommit stays changed.

@rarkins
Copy link
Collaborator

rarkins commented May 19, 2024

Keeping the logic in platform sounds better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change, requires major version bump platform:github GitHub Platform priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants