-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Handle the case when issue labels already exist #13182
Conversation
ab28ddc
to
b5b13fa
Compare
Existing acceptance tests are passing, it would be nice to add another one covering this use case.
How about we just create |
b5b13fa
to
63f5601
Compare
63f5601
to
33dd504
Compare
var testAccGitHubIssueLabelExistsConfig string = fmt.Sprintf(` | ||
// Create a repository which has the default labels | ||
resource "github_repository" "test" { | ||
name = "tf-acc-repo-label-abc1234" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need to randomize this, but apparently the other tests here aren't randomized yet either, so I'll do it in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I tried that. I used random_id, but random_id doesn't get compiled 😦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we usually generate the randomness within the test via helpers (https://github.com/hashicorp/terraform/tree/master/helper/acctest), intentionally to not couple providers like random
with other ones.
@@ -6,16 +6,24 @@ description: |- | |||
Provides a GitHub issue label resource. | |||
--- | |||
|
|||
# github\_issue_label | |||
# github_issue_label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never really understood the point of the \
escaping here in this context 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does nothing... Trust me lol. This is identical.
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. |
This fixes GH-13163
I'm not sure how to write an acceptance test for this, since I'd have to create the issue outside of the acceptance test first. Any suggestions or examples of where we do that already?