Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add printing_merge_request_link_enabled attribute to projects #783

Merged
merged 1 commit into from
Feb 14, 2022
Merged

Add printing_merge_request_link_enabled attribute to projects #783

merged 1 commit into from
Feb 14, 2022

Conversation

marceloboeira
Copy link
Contributor

As mentioned on #782, the field is available on GitLab but still not
possible to use on terraform

Reference:

marceloboeira added a commit to marceloboeira/go-gitlab that referenced this pull request Jan 17, 2022
@marceloboeira
Add PrintingMergeRequestLinkEnabled field to GET/PUT
dff2762 
Currently, the field seems to be only used on "Create" but as the
documentation shows, GET/PUT methods also have it supported.

More info:
- https://docs.gitlab.com/ee/api/projects.html
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76938
- https://github.com/gitlabhq/terraform-provider-gitlab/issues/782
- gitlabhq/terraform-provider-gitlab#783
@marceloboeira marceloboeira mentioned this pull request Jan 17, 2022
Merged
@marceloboeira
Copy link
Contributor Author

This requires xanzy/go-gitlab#1343 to be merged and released, together with an update of the go-gitlab client.

@armsnyder
Copy link
Collaborator

Thanks for the PR @marceloboeira. Once that go-gitlab feature is merged, you can feel free to upgrade it as part of this PR. Could you also add that field to the test?

@marceloboeira
Copy link
Contributor Author

marceloboeira commented Jan 17, 2022 •
edited
Loading

Could you also add that field to the test?

I just added to the default attributes of the test, I guess most tests aren't field-specific, but we can add one if that makes sense.

@armsnyder
Copy link
Collaborator

Yep, that works. Most of our tests set all the fields, and the in the subsequent "upgrade" test step they change all the fields.

@marceloboeira
Copy link
Contributor Author

UPDATE 24.01.22

The PR on gitlab client side has been merged, but still need a release so that we can use a "final" release rather than a commit sha.

Once the release is done I can update the code to point to that release and move it forward.

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

This pull request has merge conflicts. Please rebase your branch onto master.

@github-actions github-actions bot added the merge-conflict PR cannot be merged due to a merge conflict label Feb 2, 2022
@timofurrer
Copy link
Member

@marceloboeira there has been a go-gitlab release some days ago, which we've already integrated. Therefore, I think you could continue here.

Please make also sure to rebase before continuing as we've changed quite some things ;)

@timofurrer timofurrer linked an issue Feb 7, 2022 that may be closed by this pull request
Feature request: Add support for printing_merge_request_link_enabled #782
Closed
@github-actions github-actions bot added data-source Adds or modifies a data-source documentation provider resource Adds or modifies a resource tests labels Feb 7, 2022
@github-actions
Copy link

github-actions bot commented Feb 7, 2022

Conflicts are resolved. Thank you! 😀

@github-actions github-actions bot added size/S and removed merge-conflict PR cannot be merged due to a merge conflict labels Feb 7, 2022
@marceloboeira marceloboeira changed the title WIP: Add printing_merge_request_link_enabled attribute to projects Add printing_merge_request_link_enabled attribute to projects Feb 7, 2022
@marceloboeira
Copy link
Contributor Author

thanks @timofurrer, I have rebased/adjusted accordingly hehe hopefully this time is a go

@marceloboeira marceloboeira force-pushed the issue-782 branch 3 times, most recently from 1daf5ef to 48bb8e0 Compare February 7, 2022 21:09
timofurrer
timofurrer requested changes Feb 7, 2022
View reviewed changes
internal/provider/data_source_gitlab_project.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project.go Outdated Show resolved Hide resolved
internal/provider/data_source_gitlab_project.go Outdated Show resolved Hide resolved
@marceloboeira marceloboeira force-pushed the issue-782 branch 2 times, most recently from f5dc2c5 to 9a9857c Compare February 8, 2022 08:26
@armsnyder
Copy link
Collaborator

armsnyder commented Feb 8, 2022 •
edited
Loading

@marceloboeira As a tip if you didn't know, you can run make reviewable to make sure automated checks will pass.

timofurrer
timofurrer approved these changes Feb 8, 2022
View reviewed changes
Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 🎉

@marceloboeira
Copy link
Contributor Author

@timofurrer I'm going to figure out the specs failing, I believe it's something related to defaults...

@marceloboeira marceloboeira force-pushed the issue-782 branch 2 times, most recently from 4a44fb8 to 35a893a Compare February 8, 2022 19:53
@marceloboeira
Copy link
Contributor Author

@marceloboeira As a tip if you didn't know, you can run make reviewable to make sure automated checks will pass.

somehow that fails, I have to review my gosetup on a new laptop :D

@armsnyder
Copy link
Collaborator

somehow that fails, I have to review my gosetup on a new laptop :D

Could you let us know if you find out, so we can fix or add it to the CONTRIBUTING.md 🙏

@marceloboeira
Copy link
Contributor Author

somehow that fails, I have to review my gosetup on a new laptop :D

Could you let us know if you find out, so we can fix or add it to the CONTRIBUTING.md 🙏

Yes, I want to ship this one and then I will take a look if the error I have is a local problem or general incompatibility + if its worth reporting too 👍🏼

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

This pull request has merge conflicts. Please rebase your branch onto main.

@github-actions github-actions bot added the merge-conflict PR cannot be merged due to a merge conflict label Feb 9, 2022
@github-actions
Copy link

Conflicts are resolved. Thank you! 😀

@github-actions github-actions bot removed the merge-conflict PR cannot be merged due to a merge conflict label Feb 12, 2022
@marceloboeira
Add printing_merge_request_link_enabled attribute to projects
523b8f2 
As mentioned on gitlabhq#782, the field is available on GitLab but still not
possible to use on terraform

Reference:
- gitlabhq#782
@github-actions github-actions bot added size/M and removed size/S labels Feb 12, 2022
@marceloboeira
Copy link
Contributor Author

@timofurrer @armsnyder finally looking good ehhe

timofurrer
timofurrer approved these changes Feb 14, 2022
View reviewed changes
Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@armsnyder armsnyder self-requested a review February 14, 2022 08:16
@timofurrer timofurrer merged commit f27e221 into gitlabhq:main Feb 14, 2022
@marceloboeira marceloboeira deleted the issue-782 branch February 14, 2022 08:38
@armsnyder armsnyder added this to the v3.10.0 milestone Feb 14, 2022
@github-actions
Copy link

This functionality has been released in v3.10.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!

gravis pushed a commit to gravis/go-gitlab that referenced this pull request Sep 1, 2022
@marceloboeira @gravis
Add PrintingMergeRequestLinkEnabled field to GET/PUT
1103517 
Currently, the field seems to be only used on "Create" but as the
documentation shows, GET/PUT methods also have it supported.

More info:
- https://docs.gitlab.com/ee/api/projects.html
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76938
- https://github.com/gitlabhq/terraform-provider-gitlab/issues/782
- gitlabhq/terraform-provider-gitlab#783
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@armsnyder armsnyder armsnyder approved these changes

@timofurrer timofurrer timofurrer approved these changes

Assignees
No one assigned
Labels
data-source Adds or modifies a data-source documentation provider resource Adds or modifies a resource size/M tests
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

Feature request: Add support for printing_merge_request_link_enabled
3 participants
@marceloboeira @armsnyder @timofurrer