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: Move some Git commands behind a PlatformScm interface #19327

Merged
merged 22 commits into from
Feb 22, 2023

Conversation

NiasSt90
Copy link
Contributor

@NiasSt90 NiasSt90 commented Dec 9, 2022

Changes

current state: six "util/git" functions are "moved" behind the PlatformSCM interface. more to follow...

Context

Closes #19065

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 select 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

…tebot#19065

current state: six "util/git" functions are "moved" behind the PlatformSCM interface.
more to follow...
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Looks good to me, I would like to hear from @JamieMagee and @viceice about what they think of the lib/modules/platform/scm approach

@HonkingGoose
Copy link
Collaborator

Does merging this PR close this issue? Right now it's just referenced:

Or is this just one of the steps needed?

@NiasSt90
Copy link
Contributor Author

With the current state of the PR, I have successfully migrated my Gerrit implementation (local) to the PlatformScm API.
This means this is the minimum necessary extension/adaptation for supporting Gerrit (but still not complete to support/implement non-git Scm-Platforms).

I need the new util/git functions (from 2387144) and with these i could write the GerritScm like:

const gerritScm: Partial<PlatformScm> = {
  isBranchModified,
  isBranchBehindBase,
  isBranchConflicted,
  branchExists,
  getBranchCommit,
  commitAndPush: commitFiles,
  deleteBranch: (branchName: string) => Promise.resolve(), //no-op for gerrit, there exists no remote-branches for PRs
};

The first six functions are all implemented in the Gerrit platform module and rely on results from the Gerrit REST-Api.

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.

please extract the small git changes to separate PR's, they are unrelated to the scm interface and can then be merged faster.

lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
lib/util/git/index.ts Outdated Show resolved Hide resolved
lib/util/git/index.ts Outdated Show resolved Hide resolved
lib/util/git/index.ts Outdated Show resolved Hide resolved
lib/util/git/index.ts Outdated Show resolved Hide resolved
lib/util/git/index.ts Outdated Show resolved Hide resolved
NiasSt90 and others added 2 commits January 5, 2023 09:45
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
…entation

still incomplete. I don't know how to migrate this completely
(for existing configurations)?
I don't see a fully working option migration from of the
(shared-config) variable platformCommit to a global PlatformScmId configuration.
Therefore, i've added again the switch to the github scm module.
@NiasSt90 NiasSt90 requested a review from viceice February 3, 2023 14:54
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.

base classes will make future improvement more easy

lib/modules/platform/github/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/default-scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/github/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.ts Outdated Show resolved Hide resolved
# Conflicts:
#	lib/modules/manager/npm/post-update/index.ts
#	lib/workers/repository/process/extract-update.spec.ts
#	lib/workers/repository/process/extract-update.ts
lib/modules/platform/github/scm.spec.ts Outdated Show resolved Hide resolved
lib/modules/platform/github/scm.spec.ts Outdated Show resolved Hide resolved
lib/modules/platform/github/scm.ts Outdated Show resolved Hide resolved
lib/modules/platform/scm.spec.ts Outdated Show resolved Hide resolved
test/util.ts Outdated Show resolved Hide resolved
test/setup.ts Outdated Show resolved Hide resolved
lib/workers/repository/cache.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.

only the import thing left

# Conflicts:
#	lib/modules/manager/npm/post-update/index.ts
#	lib/workers/repository/process/extract-update.spec.ts
#	lib/workers/repository/process/extract-update.ts
#	lib/workers/repository/process/index.spec.ts
#	lib/workers/repository/process/index.ts
#	lib/workers/repository/update/pr/index.spec.ts
#	lib/workers/repository/update/pr/index.ts
@NiasSt90 NiasSt90 marked this pull request as ready for review February 21, 2023 07:20
lib/workers/repository/onboarding/pr/index.spec.ts Outdated Show resolved Hide resolved
test/setup.ts Outdated Show resolved Hide resolved
@viceice viceice changed the title refactor: Move all Git commands behind a PlatformScm interface #19065 refactor: Move all Git commands behind a PlatformScm interface Feb 21, 2023
@viceice
Copy link
Member

viceice commented Feb 21, 2023

@rarkins should we release as feature?

@viceice viceice requested review from rarkins, secustor and JamieMagee and removed request for rarkins February 21, 2023 12:29
@viceice viceice changed the title refactor: Move all Git commands behind a PlatformScm interface feat: Move some Git commands behind a PlatformScm interface Feb 22, 2023
@viceice viceice added this pull request to the merge queue Feb 22, 2023
Merged via the queue into renovatebot:main with commit dd6c8e5 Feb 22, 2023
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 34.150.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
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.

Move all Git commands behind a PlatformScm interface
7 participants