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

Feature: Expose updateMergeRequestInfo for gitlab api #1391

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Jul 30, 2023

Similarly to #1353, expose updateMergeRequestInfo method to be able to edit merge requests from danger code.

@glensc
Copy link
Contributor Author

glensc commented Jul 30, 2023

The CI error is not from changes of this PR:

error [email protected]: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.21.3"
error Found incompatible module.

@glensc
Copy link
Contributor Author

glensc commented Jul 30, 2023

actually, I ran such code in my danger plugin:

  await gitlab.api.MergeRequests.edit(mr.project_id, mr.iid, { assigneeId: mr.author.id })

and I'm getting 403, what's with that?

Error:  HTTPError: Response code 403 (Forbidden)
    at Request.<anonymous> (/app/node_modules/got/dist/source/as-promise/index.js:118:42)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {

EDIT: I think my token has no write accesss

@glensc
Copy link
Contributor Author

glensc commented Jul 30, 2023

@@ -290,6 +290,7 @@ export const gitlabJSONToGitLabDSL = (gl: GitLabDSL, api: GitLabAPI): GitLabDSL
fileContents: api.getFileContents,
addLabels: api.addLabels,
removeLabels: api.removeLabels,
updateMergeRequestInfo: api.updateMergeRequestInfo,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should the method be added to utils or top level (aside api: xxx)?

@orta
Copy link
Member

orta commented Jul 31, 2023

I don't think we should do this, as this fn is just an alias to this.api.MergeRequests.edit(this.repoSlug, this.prId, changes) - the other functions we expose in utils do some work under the hood - this just passes params and I think folks should be using the API instance directly for that sort of work to keep our API surface down

@glensc
Copy link
Contributor Author

glensc commented Jul 31, 2023

The wrapped methods add debug calls. it would be useful to see debug logs when troubleshooting.

But this all started because incompatible api change:

@orta
Copy link
Member

orta commented Aug 4, 2023

I'd imagine there would be similar debugging env vars on the gitlab API instance too -

yeah, gitlab.api should be the npm module! happy to switch that back over

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants