bluefeet / GitLab-API-v4

A complete GitLab API v4 client.
https://metacpan.org/pod/GitLab::API::v4
Other
20 stars 22 forks source link

Deal with end points that can not be deserialized #13

Closed royce55 closed 6 years ago

royce55 commented 6 years ago
bluefeet commented 6 years ago

Very nice! thx

bluefeet commented 6 years ago

On further inspection I ended up ditching most of the work in this pull request. Take a look at this commit for the explanation: 2fa589d5d4b98c8eba3161883e222c606e271b66

Sorry! But, thanks for inspiring me to get the REST client logic right!

bluefeet commented 6 years ago

I also wanted to comment on why I removed the return from block_user and unblock_user that you added. These two methods say:

Will return 201 OK on success, 404 User Not Found is user cannot be found or 403 Forbidden when trying to...

source: https://docs.gitlab.com/ce/api/users.html#block-user

Yes, they do return a response body with valid JSON (true), but so do many other methods which we do not return the response body from. To return it for these two methods is inconsistent with the rest of the API methods, and is misleading when the design of the GitLab::API::v4 is to throw exceptions when things fail, not return a boolean.

royce55 commented 6 years ago

I would just like to point out the behavior of Gitlab's block/unblock API calls it return true or false depending on if it actually did something. Thus if you call block on a already blocked user it returns false. I thought this was something a person might want to know.

bluefeet commented 6 years ago

Ah! Good to know! I'll add the returns back in then.

bluefeet commented 6 years ago

OK, done in d1ea411925cb01d1ff068fbc05120ebe42f64fd9.

This is such weird behavior, and its not even documented. I wonder how many other endpoints return booleans rather than failing.