New Resource: github_project_column #139
Conversation
There was a problem hiding this comment.
Hi @shihanng
thanks for this PR.
I'm generally happy to pull github_project_column in as a new resource. I left you some comments to address, but most are just small nitpicks.
Besides my inline comments, I'd greatly appreciate if you could take a brief look at my recent PR and add log messages to C/R/U/D here in a similar fashion.
I do share your feelings about github_project_column_positions.
The main reason why I think it's not a great fit for Terraform is that the definition is relative, so the declaration is not reproducible, in the sense that definition of first, last or after:<column_id> will differ in time which makes the config to produce potentially different results in different environments (with different cards). Also there doesn't seem to be any way to check the current position of a card, so we cannot detect drifts, which is one of the features of Terraform users expect from the tool.
I appreciate the time & effort you spent on the second resource (_positions), but I'm not too inclined to pull that one in, unless you or someone else comes up with a strong reason/argument.
| } | ||
|
|
||
| projectURL := column.GetProjectURL() | ||
| projectID := strings.TrimPrefix(projectURL, "https://api.github.com/projects/") |
There was a problem hiding this comment.
I know it's hard to answer without sufficient documentation or access to GitHub Enterprise instance to test this against, but do you have any idea whether it works with GHE? I reckon the prefix would differ in such case?
How do you feel about leveraging regular expression (e.g. projects/([0-9]+)$) or checking client.BaseURL?
There was a problem hiding this comment.
Also can't we use the passthrough function here instead and reuse the logic (i.e. move d.Set("project_id") there) in Read() instead? That would save us some LOC.
There was a problem hiding this comment.
I totally forgot to consider about GitHub Enterprise. b545371 addresses this comment. I've decided to use BaseURL to extract the project ID and move the logic to read. Thank you.
| if err != nil { | ||
| if resp != nil && resp.StatusCode == 404 { | ||
| d.SetId("") | ||
| return nil |
|
|
||
| # github_repository_project | ||
|
|
||
| This resource allows you to create and manage project columns for Github projects. |
There was a problem hiding this comment.
Nitpick: I'd leave out the first "project" to avoid the repetition, i.e.
This resource allows you to create and manage columns of GitHub projects.
Also please note the capital H in GitHub. 😉
| page_title: "GitHub: github_project_column" | ||
| sidebar_current: "docs-github-resource-project-column" | ||
| description: |- | ||
| Creates and manages project columns for Github projects |
There was a problem hiding this comment.
Nitpick: GitHub (capital H)
|
|
||
| The following arguments are supported: | ||
|
|
||
| * `project_id` - (Required) The id of an exisiting project that the column will be created in. |
There was a problem hiding this comment.
Typo - exisiting -> existing
There was a problem hiding this comment.
Thank you 😅. Fix in 756292e.
ghost
added
the
|
Hi @radeksimko, I've addressed your comments. Thank you so much for your time. As for I can remove/revert those if you want. |
radeksimko
force-pushed
the
project_column
branch
from
ce4b74b to
0910b34
Compare
|
@shihanng Thank you for making those changes, I squashed some commits to make the log a bit cleaner and I also removed the second |
radeksimko
changed the title
New Resource: github_project_column and github_project_column_positions
Hi, this PR adds the abilities to
github_project_columngithub_project_column_positionsinto this provider.
It uses the APIs described in https://developer.github.com/v3/projects/columns/.
I have to admit that
github_project_column_positionsis a bit odd as it does not really create/destroy any resources on GitHub but only maintains the positions of the columns. I am not sure if this violates the philosophy of Terraform.Would love to hear what you think about this. Thank you.