Skip to content

Add Project Card Support#460

Merged
jcudit merged 1 commit intomasterfrom
github_repository_project_card
Nov 23, 2020
Merged

Add Project Card Support#460
jcudit merged 1 commit intomasterfrom
github_repository_project_card

Conversation

@jcudit
Copy link
Copy Markdown
Contributor

@jcudit jcudit commented May 15, 2020

@ghost ghost added size/XL Type: Documentation Improvements or additions to documentation labels May 15, 2020
@jcudit jcudit requested a review from anGie44 May 15, 2020 03:37
@jcudit jcudit marked this pull request as ready for review May 15, 2020 03:37
anGie44
anGie44 reviewed May 15, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

nice! just a couple comments for review

github/resource_github_project_card.go Outdated
Update: resourceGithubProjectCardUpdate,
Delete: resourceGithubProjectCardDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 May 15, 2020

Choose a reason for hiding this comment

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

we'll need a custom import fn to set the card_id

github/resource_github_project_card_test.go Outdated
Note: updatedNote,
}),
),
},
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 May 15, 2020

Choose a reason for hiding this comment

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

we'll need an ImportStep here to verify but I believe the resource's Importer func will need an update as well

@jcudit
Copy link
Copy Markdown
Contributor Author

jcudit commented May 16, 2020

The following acceptance test failure was introduced:

--- FAIL: TestAccGithubProjectCard_basic (13.38s)
    testing.go:654: Step 2 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) {
        }
        
        
        (map[string]string) (len=1) {
         (string) (len=9) "column_id": (string) (len=7) "9220014"
        }
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-github/github	13.397s
FAIL

Not sure if this is a good candidate for ImportStateVerifyIgnore or if I have done something wrong with the import configuration.

@jcudit
Copy link
Copy Markdown
Contributor Author

jcudit commented May 16, 2020

@anGie44 this is ready for review again after addressing your feedback. I ended up using ImportStateVerifyIgnore to work around this error. Let me know if you have any other ideas.

@jcudit jcudit requested a review from anGie44 May 16, 2020 15:29
anGie44
anGie44 reviewed May 23, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

thanks @jcudit! just added some comments addressing the import behavior. lmk what you think as well

github/resource_github_project_card.go Outdated
}

d.Set("note", card.GetNote())
d.Set("column_id", columnID)
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 May 23, 2020

Choose a reason for hiding this comment

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

looks like this assignment is where the import step is failing b/c here columnID is an int64 while the schema is expecting a String so the field doesn't get populated.if you pass the string from L96, it should work as expected

@jcudit jcudit mentioned this pull request May 24, 2020
Merged
anGie44
anGie44 reviewed May 25, 2020
View reviewed changes
@jcudit jcudit requested a review from anGie44 May 30, 2020 21:52
@jcudit jcudit added this to the v2.9.0 milestone May 30, 2020
anGie44
anGie44 approved these changes May 30, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Output of acceptance test:

--- PASS: TestAccGithubProjectCard_basic (15.18s)

@jcudit jcudit modified the milestones: v2.9.0, v2.10.0 Jun 3, 2020
@jcudit jcudit modified the milestones: v2.10.0, v3.1.0 Jul 10, 2020
@anGie44 anGie44 modified the milestones: v3.1.0, v3.0.0, v3.2.0 Jul 31, 2020
@jcudit jcudit force-pushed the github_repository_project_card branch from ed25d6b to f0bf351 Compare November 23, 2020 14:51
@jcudit jcudit force-pushed the github_repository_project_card branch from f0bf351 to 7586306 Compare November 23, 2020 17:03
@ghost ghost added size/L and removed size/XL labels Nov 23, 2020
@jcudit
Copy link
Copy Markdown
Contributor Author

jcudit commented Nov 23, 2020

Rebased this and added passing tests.

@jcudit jcudit merged commit e9dc4e7 into master Nov 23, 2020
@jcudit jcudit deleted the github_repository_project_card branch November 23, 2020 17:09
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
Add Project Card Support (integrations#460)
24f3541
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anGie44 anGie44 anGie44 approved these changes

Assignees

No one assigned

Labels

New resource size/L Type: Documentation Improvements or additions to documentation

Projects

None yet

Milestone

v4.1.0

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add Support for Repository Project Cards

2 participants

@jcudit @anGie44