feat: GitlabComment knows how to deal with project ids #68
Conversation
`ListGitlabMergeRequests` only provides the project's id in the status, as that's what the API returns. `GitlabComment` only accepts the project's path. Not an excellent situation, but one we can readily handle. For most of the GitLab API, it will accept either a string (the "full path") or a numeric id to identify a project. Here we simply change the `Project` from a `string` to `intstr`, and inject some logic to handle it. Note that this does introduce a non-breaking CRD change.
| // +required | ||
| Project string `json:"project"` | ||
| Project *intstr.IntOrString `json:"project"` |
There was a problem hiding this comment.
I wonder why this had to be a pointer? It's required anyway, so can't be nil. It can still be explicitly "", which is the thing that needs to be checked.
There was a problem hiding this comment.
I'd thought that as well, but ultimately chose a pointer to make it consistent with GitlabMergeRequestRef.MergeRequestId -- that field is similarly required and a pointer. No objections here if you'd prefer it not be.
There was a problem hiding this comment.
Hmm true, looks like I missed that one in the past. Then I'm fine with this being a pointer as well. I'll consider "fixing" each case of this in a future api version bump.
|
@rsrchboy Thanks for the contribution, mostly LGTM :) Just one comment I added. |
codablock
changed the title
|
@rsrchboy Merged. Do you need it released asap or are you able to run with a self built container image at first? |
Thanks! I'm good for now -- I'm still in a testing stage against one repo, so self-built isn't a huge deal at the moment. |
ListGitlabMergeRequestsonly provides the project's id in the status, as that's what the API returns.GitlabCommentonly accepts the project's path.Not an excellent situation, but one we can readily handle.
For most of the GitLab API, it will accept either a string (the "full path") or a numeric id to identify a project. Here we simply change the
Projectfrom astringtointstr, and inject some logic to handle it.Note that this does introduce a non-breaking CRD change.