chore: make Gitlab.http_request() a private method#1842
chore: make Gitlab.http_request() a private method#1842JohnVillalovos wants to merge 1 commit intomainfrom
Gitlab.http_request() a private method#1842Conversation
Convert `Gitlab.http_request()` to `Gitlab._http_request()` to signify it is a private/protected method so that users of the library know they should not use the method and we make no API stability promises for using it. Add a `Gitlab.http_request()` method which will issue a Deprecation warning when called. It will pass the call onto `Gitlab._http_request()` Also, in the interest of improving code read-ability, require keyword arg usage when calling `Gitlab._http_request()`
Codecov Report
@@ Coverage Diff @@
## main #1842 +/- ##
=======================================
Coverage 92.26% 92.27%
=======================================
Files 77 77
Lines 4849 4853 +4
=======================================
+ Hits 4474 4478 +4
Misses 375 375
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| warnings.warn( | ||
| "The Gitlab.http_request() method is deprecated and will be removed in a " | ||
| "future version. This is a private method and should not be used.", | ||
| DeprecationWarning, |
There was a problem hiding this comment.
One thing about this method is that it provides a way for users to do stuff we haven't implemented (or perhaps even won't, if it's super specific). So I would maybe not be so strict about "should not be used". I would maybe say that API stability can't be guaranteed or something similar.
Edit: I've now checked a bit, because I was thinking the same that we should hide a bit more of the interface. But when looking at the original author's initial implementation, I think this might have been a deliberate choice and the distinction was more of a higher vs. lower-level API. At least what I can see from the commit message and how it was done with other private methods that were not meant to be consumed, see: c5ad540.
Multiple goals:
- Support making direct queries to the Gitlab server, without objects
and managers.
So I've started working on some docs to cover use cases here: #1846
It's a rough draft just for discussion on this, the wording and terminology is probably off and obviously "high/mid/low" can be changed to include "internal" or so.. but I think at least we should not take something away completely, unless we can cover those use cases in other ways. :)
Some use cases I vaguely remember from issues include using HEAD to check file existence/size, getting headers from the server's response, and so on. I documented some of that.
There was a problem hiding this comment.
I agree with @nejch here. It's also useful for new functionality in the GitLab API.
So let's tone the message down a bit.
Convert
Gitlab.http_request()toGitlab._http_request()to signifyit is a private/protected method so that users of the library know
they should not use the method and we make no API stability promises
for using it.
Add a
Gitlab.http_request()method which will issue a Deprecationwarning when called. It will pass the call onto
Gitlab._http_request()Also, in the interest of improving code read-ability, require keyword
arg usage when calling
Gitlab._http_request()