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

refactor(gitlab): Use new http wrapper for Gitlab platform #6275

Merged
merged 15 commits into from
May 29, 2020

Conversation

zharinov
Copy link
Collaborator

Ref: #5865

@zharinov zharinov marked this pull request as draft May 18, 2020 14:09
@zharinov
Copy link
Collaborator Author

Do we have good example repos to test against?

@zharinov
Copy link
Collaborator Author

Probably, I can just push my existing test repo to new Gitlab remote

@rarkins rarkins marked this pull request as ready for review May 20, 2020 04:32
lib/platform/gitlab/index.ts Outdated Show resolved Hide resolved
@zharinov
Copy link
Collaborator Author

Seems to be working with test repo: https://gitlab.com/zharinov/renovate-test/-/merge_requests

@rarkins rarkins requested a review from viceice May 23, 2020 05:50
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.

Some suggesttions. Maybe @zharinov can do the type optimizations in a later pr?

Comment on lines +1036 to +1042
{
iid: number;
source_branch: string;
title: string;
state: string;
created_at: string;
}[]
Copy link
Member

Choose a reason for hiding this comment

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

We sould extract a type for this

@@ -686,7 +731,7 @@ export async function setBranchStatus({

export async function getIssueList(): Promise<any[]> {
if (!config.issueList) {
const res = await api.get(
const res = await gitlabApi.getJson<{ iid: number; title: string }[]>(
Copy link
Member

Choose a reason for hiding this comment

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

Create a type for this. maybe we should move all gitlab api types to ./types.ts

const pr = (await api.get(url)).body;
const pr = (
await gitlabApi.getJson<
Pr & {
Copy link
Member

Choose a reason for hiding this comment

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

We should have a helper function for mapping Pr to have proper types, see comments below

…tlab-http-wrapper

# Conflicts:
#	lib/util/http/gitlab.spec.ts
@zharinov
Copy link
Collaborator Author

Some suggesttions. Maybe @zharinov can do the type optimizations in a later pr?

@viceice Sure, and for other platforms as well

…tlab-http-wrapper

# Conflicts:
#	lib/platform/gitlab/gl-got-wrapper.ts
…tlab-http-wrapper

# Conflicts:
#	lib/platform/gitlab/index.ts
@zharinov
Copy link
Collaborator Author

@rarkins rarkins merged commit 9617248 into renovatebot:master May 29, 2020
@rarkins rarkins deleted the feat/gitlab-http-wrapper branch May 29, 2020 05:52
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 20.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
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.

None yet

4 participants