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

provider/github Add repository deploy key resource to provider github #15215

Closed
wants to merge 2 commits into from
Closed

provider/github Add repository deploy key resource to provider github #15215

wants to merge 2 commits into from

Conversation

jtsaito
Copy link
Contributor

@jtsaito jtsaito commented Jun 9, 2017

This is PR adds a repository deploy key resource to the github provider.

This is an a slightly updated version of the WIP PR: #13576.

Copy link
Contributor

@stack72 stack72 left a comment

Choose a reason for hiding this comment

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

Hi @jtsaito

thanks for the work here - I have left a little feedback inline

Thanks

Paul

Title: &t,
}

if readOnly, ok := d.GetOk("read_only"); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a default value - therefore, there is no need for the d.GetOk("") here - it will always be there

We can replace this and do it inline in the &github.Key like Key and Title

return err
}

d.Set("key", *key.Key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not dereference these - d.Set takes care of this safely

* `repository` - (Required) Name of the Github repository.
* `title` - (Required) A title.

Changing any of the fields forces re-creating the resource.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please can you add the import section to the bottom of the docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stack72 Thanks a ton!

@stack72 stack72 added enhancement provider/github waiting-response An issue/pull request is waiting for a response from the community labels Jun 9, 2017
@jtsaito
Copy link
Contributor Author

jtsaito commented Jun 9, 2017

@stack72 Thanks for your very quick response! I addressed all three points pointed out above. Please have a look again.

@jtsaito
Copy link
Contributor Author

jtsaito commented Jun 16, 2017

@stack72 As far as I can see this PR has not been migrated to https:/terraform-providers/terraform-provider-github by the hashibot. Will this stay part of this repo or do I have to re-submit to https:/terraform-providers/terraform-provider-github?

@stack72
Copy link
Contributor

stack72 commented Jun 16, 2017

Hi @jtsaito

I can take care of migrating this now that you have fixed it up as expected

Thanks for the work here

Paul

@stack72
Copy link
Contributor

stack72 commented Jun 16, 2017

Hi @jtsaito

I just used the git patch to push this to the other repo - thanks for the work here :)

Paul

@stack72 stack72 closed this Jun 16, 2017
@jtsaito
Copy link
Contributor Author

jtsaito commented Jun 16, 2017

@stack72 Thanks!

@fmasuhr fmasuhr deleted the github-repository-delpoykeys branch June 16, 2017 14:05
@ghost
Copy link

ghost commented Apr 8, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/github waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants