New Resource: github_organization_project #111
Conversation
radeksimko
changed the title
|
It seems that there is conflict. Would you like me to rebase on master? |
|
@shihanng yes, I merged a few PRs lately and I'm going through others, including this one. I'm going to review it regardless, but rebasing would be incredibly helpful. Thanks! |
| "url": { | ||
| Type: schema.TypeString, | ||
| Computed: true, | ||
| }, |
There was a problem hiding this comment.
How do you feel about coverage of the remaining fields which are available through PATCH?
https://developer.github.com/v3/projects/#update-a-project
specifically state, organization_permission and public?
There was a problem hiding this comment.
🤔 It seems that the library that we are using does now support these fields yet?
- There is a way to set the
stateviaProjectOptionsbut the value of thestateis not included inProjectso we can't get the current state of the project via the library. - For
organization_permission, is seems that there is an issue opened and being worked on currently Support preview Organization Project Permissions API google/go-github#884 - For
public, is it not available via the go-github library yet.
There was a problem hiding this comment.
Ah, understood. Thanks for the detailed answer & research. We shall leave it out for the initial implementation then.
| options := github.ProjectOptions{ | ||
| Name: n, | ||
| Body: b, | ||
| } |
There was a problem hiding this comment.
Nitpick (non-blocking): I think we could just inline the fields above to the struct to reduce LOC and improve readability a bit.
There was a problem hiding this comment.
Fixed in f1a04f4, thank you.
| options := github.ProjectOptions{ | ||
| Name: n, | ||
| Body: b, | ||
| } |
There was a problem hiding this comment.
Fixed in f1a04f4, thank you.
|
|
||
| * `name` - The name of the project. | ||
|
|
||
| * `body` - The body of the project. |
There was a problem hiding this comment.
Can you please document whether each field is Optional or Required here?
There was a problem hiding this comment.
Added in 2ea7426. Thanks!
| }, | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you mind adding a test for import also?
There was a problem hiding this comment.
Added test for import in ec08678. Thank you.
|
|
||
| d.Set("name", project.GetName()) | ||
| d.Set("body", project.GetBody()) | ||
| d.Set("url", fmt.Sprintf("https://github.com/orgs/%s/projects/%d", o, project.GetNumber())) |
There was a problem hiding this comment.
Is there any reason we should be constructing the URL here instead of reading it from the API? It appears that html_url is the attribute we could use?
There was a problem hiding this comment.
Ah yes, but it is not made available in the go-github library? See Project.
shihanng
force-pushed
the
organization_project
branch
from
4911f9b to
8972a54
Compare
shihanng
force-pushed
the
organization_project
branch
from
8972a54 to
2ea7426
Compare
|
@radeksimko, thank you for the reviews. I've addressed some comments in code, and also the rest as comments. Also rebased on master. Could you please take another look. Thank you. |
There was a problem hiding this comment.
LGTM 👍
Thank you for making all the changes promptly.
FYI I have opened google/go-github#969 to add the missing fields you mentioned, so we can update the resource code later, when it lands.
|
Thank you so much 😄! I will work on #115 later on and ping you again when it is ready. |
* r/github_organization_project: first commit * r/github_organization_project: add docs * add import test for github_organization_project * inline struct fields for readability * docs: update fields with Required/Optional
Hi, this PR adds the ability to manage organization projects to this provider as described in https://developer.github.com/v3/projects/#create-an-organization-project. This is my first time working with Terraform provider, please let me know if I miss anything. Thank you.