-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gitlab_project: add param "issues_access_level" #8760
gitlab_project: add param "issues_access_level" #8760
Conversation
15391f4
to
cded9cf
Compare
@julien-lecomte you can now rebase this PR! 😉 |
cded9cf
to
e6b408d
Compare
@lgatellier Branch has been rebased. |
Thanks for your contribution! I think both options (the new one and the deprecated one) should be mutually exclusive, to avoid sending two incompatible options values to the API. |
e6b408d
to
7ee69da
Compare
The mutual exclusive flag was added. |
7ee69da
to
cce12d0
Compare
cce12d0
to
e085a69
Compare
I've removed that occurrence and another one in the same time. |
e085a69
to
441cdfe
Compare
@lgatellier I see that issues_enabled defaults to True. This is probably wrong now, since it would activate issues whenever neither is specified. Others (wiki_enabled) also default to True. I believe that these values should have no default. |
I agree it should not have a default, but for some reason it (and some other options) have one. Getting rid of defaults is not so easy, you have to deprecate them first and eventually remove them. For that I would add a new option that allows to disable them, and deprecate that option's default instead. As an example of how that goes, see #850 for adding such an option (that option's default changed in 4.0.0, and the option was eventually removed in #6836). |
In such a case, I think it's best to leave that to a future PR. |
If nobody objects, I'll merge this at the upcoming weekend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@julien-lecomte thanks for your contribution! |
@lgatellier @russoz thanks for reviewing! |
Backport to stable-9: 💚 backport PR created✅ Backport PR branch: Backported as #8805 🤖 @patchback |
gitlab_project: add option `issues_access_level` to enable/disable project (cherry picked from commit 5192ffe)
…ues_access_level" (#8805) gitlab_project: add param "issues_access_level" (#8760) gitlab_project: add option `issues_access_level` to enable/disable project (cherry picked from commit 5192ffe) Co-authored-by: Julien Lecomte <[email protected]>
…8760) gitlab_project: add option `issues_access_level` to enable/disable project
…8760) gitlab_project: add option `issues_access_level` to enable/disable project
SUMMARY
There already is
issues_enabled
, but this seems to be the old name.issues_access_level
seems to be the new name.The first one is a bool, and the second one has the usual ["private", "disabled", "enabled"] values.
There are currently no handling of parameters depending on the gitlab version, and thus no code of such sort has been added to handle old (very old?) versions of gitlab. Nevertheless, new versions of gitlab don't seem bothered with obsolete values such as
issues_enabled
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
This pull request is a commit above the one in #8759
For this reason, it is expected that #8759 be merged before this one being merged.