Codecov Report

Merging#1278 (2edd69c) intomaster (643454c) willincrease coverage by0.00%.
The diff coverage is100.00%.

@@           Coverage Diff           @@##           master    #1278   +/-   ##=======================================  Coverage   78.19%   78.19%           =======================================  Files          12       12             Lines        2884     2885    +1     =======================================+ Hits         2255     2256    +1  Misses        629      629

Flags with carried forward coverage won't be shown.Click here to find out more.

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update643454c...2edd69c. Read thecomment docs.

If you do decide to make a breaking change I wonder if requiring kwargs would be a good idea?

def __init__(    self,    *,    url,    private_token=None,    oauth_token=None,    job_token=None,    ssl_verify=True,    http_username=None,    http_password=None,    timeout=None,    api_version="4",    session=None,    per_page=None,    pagination=None,    order_by=None,    user_agent=gitlab.const.USER_AGENT,):

And okay if people think it is a terrible idea 🙂 Just thought I would mention it.

@JohnVillalovos Not sure what you mean byrequiring kwargs would be a good idea. Could you elaborate?

@JohnVillalovos Not sure what you mean byrequiring kwargs would be a good idea. Could you elaborate?

Sure. So by using the* in the argument list it will require people to use keyword arguments.

Allowed:

gl = gitlab.Gitlab(url="gitlab.com")

Not allowed:

gl = gitlab.Gitlab("gitlab.com")TypeError: __init__() takes 1 positional argument but 2 were given

It is a breaking change for sure to do that.

@JohnVillalovos I'm not so sure. I'm not really a fan of this change. This would really break everything.

@JohnVillalovos I'm not so sure. I'm not really a fan of this change. This would really break everything.

No worries. As a note you can put the* where ever you like in the arguments list. So if you wanted to require keyword arguments for everything buturl you could put the* after theurl argument.

def __init__(    self,    url,    *,    private_token=None,    oauth_token=None,    job_token=None,    ssl_verify=True,    http_username=None,    http_password=None,    timeout=None,    api_version="4",    session=None,    per_page=None,    pagination=None,    order_by=None,    user_agent=gitlab.const.USER_AGENT,):

@JohnVillalovos I'm not so sure. I'm not really a fan of this change. This would really break everything.

No worries. As a note you can put the* where ever you like in the arguments list. So if you wanted to require keyword arguments for everything buturl you could put the* after theurl argument.

def __init__(    self,    url,    *,    private_token=None,    oauth_token=None,    job_token=None,    ssl_verify=True,    http_username=None,    http_password=None,    timeout=None,    api_version="4",    session=None,    per_page=None,    pagination=None,    order_by=None,    user_agent=gitlab.const.USER_AGENT,):

I agree with Max I've seen a lot of use of this in tiny scripts, and there people probably want to use quick one-liners without kwargs just to instantiateGitlab with a private token, so I'd keep this as is for now.

@JohnVillalovos while resolving conflicts I consolidated the URL redirect warnings as I saw they were in 3 places, in case you'd like to have 1 final look, but this should also be ready now.