Skip to content

[Feature Request] github milestone resource and data source#470

Merged
jcudit merged 4 commits intomasterfrom
ap_milestones_support
Nov 23, 2020
Merged

[Feature Request] github milestone resource and data source#470
jcudit merged 4 commits intomasterfrom
ap_milestones_support

Conversation

@anGie44
Copy link
Copy Markdown
Collaborator

@anGie44 anGie44 commented May 24, 2020

Closes #295

Changes include:

  • new resource/data-source: github_repository_milestone
  • import support with: owner/repository/number

Output of acceptance tests:

--- PASS: TestAccGithubRepositoryMilestone_disappears (13.65s)
--- PASS: TestAccGithubRepositoryMilestone_update (17.49s)
--- PASS: TestAccGithubRepositoryMilestone_basic (17.65s)
--- PASS: TestAccGithubRepositoryMilestone_multiple (21.82s)

--- PASS: TestAccGithubRepositoryMilestoneDataSource_noMatchReturnsError (0.34s)
--- PASS: TestAccGithubRepositoryMilestoneDataSource_existing (8.96s)

@ghost ghost added size/XL Type: Documentation Improvements or additions to documentation labels May 24, 2020
@anGie44 anGie44 changed the title add milestone resource and data source [Feature Request] github milestone resource and data source May 24, 2020
jcudit
jcudit approved these changes May 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jcudit jcudit left a comment

Choose a reason for hiding this comment

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

Couple of suggestions but nothing to block on.

github/data_source_github_repository_milestone.go
return err
}

d.SetId(strconv.FormatInt(milestone.GetID(), 10))
Copy link
Copy Markdown
Contributor

@jcudit jcudit May 24, 2020

Choose a reason for hiding this comment

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

What are your thoughts on new resources using the global node IDs when SetId() is being called?

Previous experimentation in https://github.com/terraform-providers/terraform-provider-github/pull/460 takes this approach:

https://github.com/terraform-providers/terraform-provider-github/blob/23c6faefa7da8039b9015e4c85b154a7928f685b/github/resource_github_project_card.go#L67

https://github.com/terraform-providers/terraform-provider-github/blob/23c6faefa7da8039b9015e4c85b154a7928f685b/github/resource_github_project_card_test.go#L63

github/resource_github_repository_milestone.go

func resourceGithubRepositoryMilestoneCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*Organization).v3client
ctx := context.Background()
Copy link
Copy Markdown
Contributor

@jcudit jcudit May 24, 2020

Choose a reason for hiding this comment

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

Recently came across this:

https://github.com/terraform-providers/terraform-provider-github/blob/518829b4f87f8613feaa86b4046702a962619830/github/provider.go#L136

Does the provider become more responsive to a cancel operation if we wire this context all the way through to requests like this?

@jcudit jcudit modified the milestones: v2.10.0, v2.9.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.2.0 Jul 31, 2020
@ewilde
Copy link
Copy Markdown
Contributor

ewilde commented Nov 13, 2020

@jcudit is there anything left to do on this PR. I'm happy to help out if it's missing anything 👍

@jcudit jcudit force-pushed the ap_milestones_support branch from 518829b to 41446df Compare November 22, 2020 20:27
anGie44 and others added 4 commits November 23, 2020 09:46
@jcudit jcudit force-pushed the ap_milestones_support branch from 6c0afde to 94c06b9 Compare November 23, 2020 14:46
@jcudit
Copy link
Copy Markdown
Contributor

jcudit commented Nov 23, 2020

@ewilde thanks for the ping on this one 😄

This has been rebased and the tests have been brought in line with the latest suite. Look out for an upcoming CHANGELOG PR and raise any issues once this releases. Happy to work with any data you can bring back on the next bugfix release if this doesn't play well with your use case.

@jcudit jcudit merged commit 35b1a36 into master Nov 23, 2020
@jcudit jcudit deleted the ap_milestones_support branch November 23, 2020 14:49
@ewilde
Copy link
Copy Markdown
Contributor

ewilde commented Nov 23, 2020

@jcudit fantastic, thank you for taking the time to get this feature into the provider 👍 It looks like it will be a good match for my basic use case

kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
@anGie44
[Feature Request] github milestone resource and data source (integrat…
988b3ec 
…ions#470)

* add milestone resource and data source

* fix misspell

* rebase and fix build errors

- update to go-github v32
- use `Owner` instead of `Organization`

* update test suite

Co-authored-by: Jeremy Udit <jcudit@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jcudit jcudit jcudit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/XL 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] Add milestone support

3 participants

@anGie44 @ewilde @jcudit