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.

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

Implement gitlab service slack #96

Merged
merged 8 commits into from May 1, 2019

Conversation

jorcau
Copy link
Collaborator

@jorcau jorcau commented Mar 4, 2019

Hi,

Here is a new proposal for Slack Integration implementation. It reuses work started on #65. I've talked with @hypnoglow about doing another PR so I guess we could close #65 for this one.

It includes documentation and acceptance tests:

➜ make testacc TEST=./gitlab TESTARGS='-run=TestAccGitlabServiceSlack_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./gitlab -v -run=TestAccGitlabServiceSlack_* -timeout 120m
=== RUN   TestAccGitlabServiceSlack_basic
--- PASS: TestAccGitlabServiceSlack_basic (8.23s)
=== RUN   TestAccGitlabServiceSlack_import
--- PASS: TestAccGitlabServiceSlack_import (6.95s)
PASS
ok  	github.com/terraform-providers/terraform-provider-gitlab/gitlab	15.209s

Thanks,

@roidelapluie
Copy link
Collaborator

FYI now the Acc tests are run in travis too.

@roidelapluie
Copy link
Collaborator

roidelapluie commented Mar 4, 2019

Can you please add an ACC test with default values? Current tests are great but too explicit. I would like another, minimalist one.

@jorcau
Copy link
Collaborator Author

jorcau commented Mar 4, 2019

FYI now the Acc tests are run in travis too.

This is awesome!

I added a test with only required fields set as you asked.

@jorcau jorcau force-pushed the implement-gitlab-service-slack branch from c3e4c15 to 0295fa0 Compare March 5, 2019 17:31

func resourceGitlabServiceSlackImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
d.Set("project", d.Id())

Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldnt we fetch the service at this point from gitlab?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure what you mean here.

From what I understand, the resourceGitlabServiceSlackImportState function should just define the ID or necessary parameters so the READ function can then be called to actually fetch the resource's details.

@jorcau jorcau force-pushed the implement-gitlab-service-slack branch from 0295fa0 to 6dfd2cb Compare March 8, 2019 15:36
@rsrchboy
Copy link

rsrchboy commented Apr 3, 2019

@roidelapluie Are there any blockers to merging this?

@jorcau
Copy link
Collaborator Author

jorcau commented Apr 30, 2019

Hi @roidelapluie, do you have time to review & merge this anytime soon? Thanks!

@roidelapluie roidelapluie merged commit 959dabd into gitlabhq:master May 1, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
4 participants