Skip to content
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

feat(branch-protection): Push/Reviewer actors can be specified by name #1020

Merged
merged 6 commits into from
Oct 12, 2022
Merged

feat(branch-protection): Push/Reviewer actors can be specified by name #1020

merged 6 commits into from
Oct 12, 2022

Conversation

dion-gionet
Copy link
Contributor

Changes the github_branch_protection resource.

This enables us to use the branch protection resource without using a data block just to retrieve the node ID for users/teams. Node IDs can still be used as an input but you can now use a prefix to define a username or team name

The / prefix defines a user (Ex.: /testuser), the orgname/ prefix defines a team. (Ex.: testorg/team)

feat: Added getNodeIDv4

feat(branch-protection): Added prefix requirement for teams/users

The / prefix defines a user, the orgname/ prefix defines a team. (Ex.: testorg/team)
includes a nil check on branch protection resources
@dion-gionet
Copy link
Contributor Author

@kfcampbell Do you have any feedback on this?

github/util_v4_branch_protection.go Outdated Show resolved Hide resolved
github/util_v4_branch_protection.go Outdated Show resolved Hide resolved
github/util_v4_branch_protection.go Outdated Show resolved Hide resolved
github/util_v4_branch_protection.go Outdated Show resolved Hide resolved
github/util_v4_branch_protection.go Show resolved Hide resolved
@dion-gionet
Copy link
Contributor Author

@kfcampbell I've made some changes with the feedback you gave, let me know if you have any more recommendations :)

Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

Thanks for updating this! Integration tests for this branch protection resource are broken on the main branch and failing here as well with this error:

    testing.go:705: Step 0 error: errors during apply:
        
        Error: Could not resolve to a node with the global id of '1234'
        
          on /tmp/tf-test2340247973/main.tf line 8:
          (source code not available)

So I don't think this is a regression. Thank you for the contribution.

@kfcampbell
Copy link
Member

@dion-gionet Are you okay if I merge/release this PR today?

@dion-gionet
Copy link
Contributor Author

@kfcampbell Yep, sounds good!

@kfcampbell kfcampbell merged commit 553785b into integrations:main Oct 12, 2022
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Oct 13, 2022
kfcampbell added a commit that referenced this pull request Oct 13, 2022
@dion-gionet dion-gionet deleted the branch-protection-string-1 branch October 28, 2022 16:41
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
integrations#1020)

* feat(branch-protection): Push/Reviewer actors can be specified by name

feat: Added getNodeIDv4

feat(branch-protection): Added prefix requirement for teams/users

The / prefix defines a user, the orgname/ prefix defines a team. (Ex.: testorg/team)

* test(branch-protection): Added push restriction test with username

* doc(branch-protection): Updated documentation to mention actor names

* feat(branch-protection): updated to latest version

* fix: env nil pointer dereference

includes a nil check on branch protection resources

* fix(branch-protection): added error handling

Also removed loop labels
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
integrations#1020)

* feat(branch-protection): Push/Reviewer actors can be specified by name

feat: Added getNodeIDv4

feat(branch-protection): Added prefix requirement for teams/users

The / prefix defines a user, the orgname/ prefix defines a team. (Ex.: testorg/team)

* test(branch-protection): Added push restriction test with username

* doc(branch-protection): Updated documentation to mention actor names

* feat(branch-protection): updated to latest version

* fix: env nil pointer dereference

includes a nil check on branch protection resources

* fix(branch-protection): added error handling

Also removed loop labels
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants