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

Add installation instructions #419

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Conversation

guineveresaenger
Copy link
Contributor

@guineveresaenger guineveresaenger commented Sep 21, 2024

Part of pulumi/home#3598

Generates a provider-side docs/_index.md file with relevant content.

There is an additional text find/replace that is expressed in in docs/index-md-replaces, which will fail with a hard error when the input text is no longer found upstream.

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

@guineveresaenger guineveresaenger marked this pull request as ready for review October 29, 2024 20:58
@guineveresaenger guineveresaenger requested a review from a team October 29, 2024 20:59
Comment on lines +180 to +181
"Configuring Multiple Tokens",
"Multi-Region Deployments",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should skip these sections? Nomad is a hashicorp service, so I assume folks using this provider may need the instructions for working with vault/consul tokens and different regions?

I bet these break because we don't support converting multiple provider configs in a single hcl example though right? But if that's the reason not to include these sections, maybe we should link to a converter issue in the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not just that. These sections specifically discuss provider aliases and terraform variables in the text which pushed me over the edge of eliding these.

Correcting this would be a fairly heavy lift rewrite, just to show that each region needs their own token, and that you can set Vault and Consul tokens also on different providers. I can do this, but the cost might be that this will break more upgrades. The file itself doesn't see a lot of action over time so this may be worth it.

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with just dropping these for now.

Comment on lines +180 to +181
"Configuring Multiple Tokens",
"Multi-Region Deployments",
Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with just dropping these for now.

@guineveresaenger guineveresaenger added the needs-release/patch When a PR with this label merges, it initiates a release of vX.Y.Z+1 label Oct 30, 2024
@guineveresaenger guineveresaenger merged commit b568f44 into master Oct 30, 2024
23 checks passed
@guineveresaenger guineveresaenger deleted the guin/installation-docs branch October 30, 2024 21:10
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v2.4.1.

@github-actions github-actions bot removed the needs-release/patch When a PR with this label merges, it initiates a release of vX.Y.Z+1 label Oct 30, 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.

3 participants