-
Notifications
You must be signed in to change notification settings - Fork 319
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
Add Project-level MR approvals #356
Add Project-level MR approvals #356
Conversation
Any idea why
|
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.
Looks great! A few comments.
239af25
to
1c08bc2
Compare
Can we run |
Shouldn't this be part of the |
@horjulf totally makes sense but the API doesn't support it https://docs.gitlab.com/ee/api/projects.html#create-project |
I see, that does make it tricky to manage in a single resource 🤔 |
Do you need any help with this one? I'd really like to see this released. I can have a look at your failing test! |
@jeremad feel free to contribute. I’ ve tested the module and the tests on a Gitlab EE but it fails in Github actions despite the SkipFunc which should be skipping tests for community edition and run for EE only. |
I think it is expected that the tests run but they should be skipped immediately and only take a few ms (at least that's what happens with GitHub tests on master: https://github.com/terraform-providers/terraform-provider-gitlab/runs/936184044?check_suite_focus=true) |
You also need to add your new doc page to |
I fixed the review comments, squashed the commits and rebased on master I can open another MR, or you can push force that here: |
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. The only remaining thing would be changing project_id
type to TypeInt
, but that's minor.
Do we need additional approvals before merge? Adding @nicholasklick |
Looks like this needs rebasing @husunal |
4be2970
to
a8ad4ad
Compare
Thanks, rebased @nicholasklick @armsnyder |
Thanks! This looks ready to merge @nicholasklick |
Initially introduced in gitlabhq#356
This MR adds a Project-level MR approvals resource.