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.

Sign up for GitHub

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

Jump to bottom

README.md Markdown link typo fix #20

Merged
merged 2 commits into from
Jun 29, 2020
Merged

README.md Markdown link typo fix #20

merged 2 commits into from
Jun 29, 2020

Conversation

thenaturalist
Copy link
Contributor

This commit fixes a Markdown link typo and includes link checking for references.

soerenmartius
soerenmartius requested changes Jun 26, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
mariux
mariux requested changes Jun 27, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
@thenaturalist
README.md Markdown link typo fix
9b05f11 
Correct link reference & adjust Markown link checking
@mariux
Copy link
Member

mariux commented Jun 29, 2020

github provider 2.9.0 seems to be broken as token is required now...

@soerenmartius
Copy link
Member

github provider 2.9.0 seems to be broken as token is required now...

The token was always required by the provider? It doesn't make much sense without the token anyways, does it ?

But yeah, pre-commit fails because of the missing token: https://mineiros.semaphoreci.com/jobs/9157e276-7ff7-4e3f-abbe-86eb16549329

@mariux
Copy link
Member

mariux commented Jun 29, 2020

we specify the token vie environment so this was fine before..

@soerenmartius
Copy link
Member

soerenmartius commented Jun 29, 2020 •
edited
Loading

we specify the token vie environment so this was fine before..

Yeah, we are providing it through the GITHUB_TOKEN environment variable

@thenaturalist
Adapt pre-commit hook tests to include GitHub token
a9dfed4 
This change adapts to a provider change requiring a GitHub token to be
set when running terraform validate.
@mariux
Copy link
Member

mariux commented Jun 29, 2020

The token was always required by the provider? It doesn't make much sense without the token anyways, does it ?

See https://github.com/terraform-providers/terraform-provider-github/pull/464#issuecomment-651254293

As the provider was refactored to not only support organizations, the token went back to the required state. Setting the GITHUB_TOKEN environment variable to any string will fix terraform validate. tested and confirmed it with 2.8.0 and 2.9.0 of the provider.

We should also check out what this refactoring means for the features of our GitHub modules.. maybe we can add some awesome stuff...maybe we can't ;)

The fix you applied for providing the variable needs to be integrated into the other two repositories. I can create PRs for this. thanks.

mariux
mariux approved these changes Jun 29, 2020
View reviewed changes
Copy link
Member

@mariux mariux left a comment

Choose a reason for hiding this comment

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

lgtm

@thenaturalist thenaturalist merged commit ad54544 into master Jun 29, 2020
@thenaturalist thenaturalist deleted the jan/spellcheck branch June 29, 2020 18:57
@mariux
Copy link
Member

mariux commented Jun 30, 2020

I raised the issue upstream: https://github.com/terraform-providers/terraform-provider-github/issues/503
don't think we need further steps on our side as this is fixed now by providing the token.
It still breaks local pre-commits when no token is set, but it can be set to a random value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soerenmartius soerenmartius soerenmartius requested changes

@mariux mariux mariux approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thenaturalist @mariux @soerenmartius