Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

Terraform GitHub Actions updates #988

Merged
merged 7 commits into from
Nov 13, 2019
Merged

Terraform GitHub Actions updates #988

merged 7 commits into from
Nov 13, 2019

Conversation

sudomateo
Copy link
Contributor

Updated to reflect YAML syntax
Reorganized content layout

@sudomateo sudomateo requested a review from a team as a code owner November 5, 2019 06:30
@sudomateo
Copy link
Contributor Author

I meant to make this a draft pull request. I still have a few updates I would like to make.

@sudomateo sudomateo removed request for a team and nfagerlund November 6, 2019 15:10
@sudomateo sudomateo changed the title Terraform GitHub Actions updates [WIP] Terraform GitHub Actions updates Nov 6, 2019
@sudomateo
Copy link
Contributor Author

This is ready for review.

@sudomateo sudomateo changed the title [WIP] Terraform GitHub Actions updates Terraform GitHub Actions updates Nov 8, 2019
Copy link
Member

@nfagerlund nfagerlund left a comment

Choose a reason for hiding this comment

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

This is an impressive rewrite; thank you for it!

There are a few things inline that I think need adjustments. There are also two structural things that need a bit more iteration:

Sections

The name of the "Configuration" section makes sense. But the "Workflows" section is confusing, because it collides with the local term of art, where in GitHub Actions a "workflow" is a YAML file that sets up a chain of actions. Consequently, the "Workflows" section doesn't contain what I'd expect it to, after reading the front page and the main configuration page:

image

In a different context, I could get down with "workflows" being the correct name for what we're dealing with here, but locally I think we have to call it something different. Spitballing here:

  • How-To
  • Customization
  • Common Tasks

...but I'm open to other suggestions.

If you don't want to wrestle with this one alone, we can jam about it in the #proj-tf-docs channel or something.

Redirects

This changes a bunch of URLs! Before we ship it, we've gotta get the appropriate redirects into content/redirects.txt.

That should probably wait until we sort out what to do about the workflows directory.

@sudomateo
Copy link
Contributor Author

Updated with changes based on your feedback. I went with the name "Common Actions" instead of "Workflows". I'm open to changing it if needed.

I also updated content/redirects.txt to redirect all of the changed URLs to their new location.

@sudomateo
Copy link
Contributor Author

The TravisCI build is complaining about missing links.

http://127.0.0.1:4567/docs/providers/azurerm/auth/azure_cli.html:
Remote file does not exist -- broken link!!!
http://127.0.0.1:4567/docs/providers/azurerm/auth/managed_service_identity.html:
Remote file does not exist -- broken link!!!
http://127.0.0.1:4567/docs/providers/azurerm/auth/service_principal_client_secret.html:
Remote file does not exist -- broken link!!!

Seems like these links are now located at the following places respectively.

/docs/providers/azurerm/guides/azure_cli.html
/docs/providers/azurerm/guides/managed_service_identity.html
/docs/providers/azurerm/auth/service_principal_client_secret.html

@nfagerlund
Copy link
Member

Looking into those azure things right now, I think I found the issue.

Copy link
Member

@nfagerlund nfagerlund left a comment

Choose a reason for hiding this comment

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

For any spectators: we caught up in Slack and are gonna change it to "Common Tasks."

Here's one or two more small items I noticed — ping me once you're done, and I'll do a last check to make sure all the links line up, and then we can :shipit:

sudomateo and others added 7 commits November 13, 2019 11:06
Updated to reflect YAML syntax
Reorganized content layout
Fixing links to adhere to layout changes.
Renamed Common Actions to Common tasks and removed `sidebar_current`
from front matter within the Markdown files.
Copy link
Member

@nfagerlund nfagerlund left a comment

Choose a reason for hiding this comment

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

Thanks again for all your work on this! Tweaked the thing about outputs to reference user-level docs, and added an example. And with that, I think we're done.

@nfagerlund nfagerlund merged commit 4339be5 into master Nov 13, 2019
@sudomateo sudomateo deleted the docs/github-actions branch February 8, 2020 00:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants