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

Remove hardcoded github provider #40

Merged
merged 11 commits into from
Mar 23, 2023
Merged

Conversation

drselump14
Copy link
Contributor

@drselump14 drselump14 commented Nov 21, 2022

  • Remove detailed GitHub provider
  • Allow GitHub provider to be injected

Resolves #34
Follows up for PR #35

@drselump14 drselump14 changed the title master Remove provider github Nov 21, 2022
@nitrocode
Copy link
Member

/test all

@nitrocode
Copy link
Member

@drselump14 please add the repo root's versions.tf to examples/complete directory to fix test/bats and get the test/terratest to run.

@nitrocode
Copy link
Member

/test all

@nitrocode
Copy link
Member

/test all

versions.tf Show resolved Hide resolved
@drselump14 drselump14 requested review from nitrocode and removed request for dotCipher and korenyoni November 21, 2022 04:52
@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode requested a review from Nuru November 21, 2022 05:19
@nitrocode
Copy link
Member

/test all

Comment on lines 18 to 21

providers = {
github = github
}
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed since there isn't an alias, the provider should be implicitly provided

@nitrocode
Copy link
Member

There is a terratest error. Looks like it doesn't show an org in the api call which is odd since you're correctly passing in the owner and the integrations/github provider states in their docs that the organization input is deprecated.

Error: POST https://api.github.com/repos//terraform-github-repository-webhooks/hooks: 404 Not Found []

I'm unsure why the org is missing here.

Please troubleshoot and run the tests locally to reproduce and resolve.

@drselump14
Copy link
Contributor Author

drselump14 commented Nov 21, 2022

There is a terratest error. Looks like it doesn't show an org in the api call which is odd since you're correctly passing in the owner and the integrations/github provider states in their docs that the organization input is deprecated.

Error: POST https://api.github.com/repos//terraform-github-repository-webhooks/hooks: 404 Not Found []

I'm unsure why the org is missing here.

Please troubleshoot and run the tests locally to reproduce and resolve.

I'm unsure how to run terratest locally, but I can confirm that terraform apply --var-file fixtures.us-east-2.tfvars works with the latest commit.

@nitrocode
Copy link
Member

/test all

@drselump14 drselump14 changed the title Remove provider github Remove hardcoded github provider Nov 21, 2022
@nitrocode
Copy link
Member

cc: @Nuru please review when you get a chance. Requesting your help since you gave an in-depth set of steps in the previous PR and @drselump14 has taken it on to complete each one.

The tests pass and so far this looks pretty good to me. I think a second set of eyes would help here.

Copy link
Member

@nitrocode nitrocode left a comment

Choose a reason for hiding this comment

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

Nice job!

@mihaiplesa
Copy link

Looking forward to this.

@adamantike
Copy link

adamantike commented Mar 23, 2023

Can this change be considered for merge and release? It would also fix #37, which is avoiding higher-level modules (like cloudposse/terraform-aws-ecs-web-app) from using count or for_each.

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.

GitHub provider not supported on arm64 macs (M1)
5 participants