GitlabAPI#getCommits returns branch commits #259

Activity

1. maxim5 commented on Oct 16, 2017

   

   maxim5

   on Oct 16, 2017

   Last edited by maxim5

   Author

   More actions

   Looks like v3 API is very different from v4: /commits and /changes accept merge request id, not iid.
   Unfortunately, GitlabAPI hard-codes v3 and is not extensible, so I can't use v4 with current implementation.
2. Fractaliste commented on Dec 3, 2018

   

   Fractaliste

   on Dec 3, 2018

   More actions

   I got the same when getting commit from a Tag. It's quiet annoying because it prevents me to get author's name...
3. rkpatel7 commented on Oct 11, 2019

   

   rkpatel7

   on Oct 11, 2019

   Contributor

   More actions

   Since #333, it's been possible to pass in an apiNameSpace but the default is now v4.

   Regardless, in both v3 and v4, Gitlab provides a way to get commits associated with an MR:

   • v3: GET /projects/:id/merge_requests/:merge_request_id/commits
   • v4: GET /projects/:id/merge_requests/:merge_request_iid/commits

   Given that it's very similar other than merge_request_id vs merge_request_iid and all the other methods in this library right now are using iid, I think it's safe to update the implementation for getCommits to use the correct endpoint.

   I'll go ahead and submit a PR for this as I have had to implement a workaround for this in the past and would now like to fix it in this library instead of using my workaround. @timols if you have any thoughts/concerns, please let me know.

   @Fractaliste I would suggest opening a separate issue for getting commits from a Tag since I would like to keep the scope of this change limited to the original issue.
4. rkpatel7

   added a commit that references this issue on Oct 14, 2019

   Update getCommits for merge requests to use the correct gitlab API en…

   ...

   This issue will close once commit 5f7cb84 is merged into the 'master' branch.

   5f7cb84
5. timols

   closed this as completedin #361on Oct 18, 2019
6. timols

   added a commit that references this issue on Oct 18, 2019

   Get commits for merge request (#361)

   ...

   This issue will close once commit 2863c78 is merged into the 'master' branch.

   2863c78