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

Docs: Refactor "Environment variables" into 3 articles (Diátaxis) #9966

Merged

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented Jan 31, 2023

This breaks up our current Environment Variables guide:

  • Avoids mix of "pre-defined" environment reference with "user-defined" scope
  • Replaces "user-defined" with "custom"
  • Adds more guidance in howto + a screenshot
  • Discusses alternatives to non-secret environment variables

Refs: #9746


📚 Documentation previews 📚

@benjaoming benjaoming added Improvement Minor improvement to code Needed: documentation Documentation is required labels Jan 31, 2023
@benjaoming benjaoming changed the base branch from main to diataxis/main January 31, 2023 15:16
@benjaoming benjaoming marked this pull request as ready for review February 1, 2023 16:30
@benjaoming benjaoming requested a review from a team as a code owner February 1, 2023 16:30
@benjaoming
Copy link
Contributor Author

@ericholscher I dare say that this is ready for review 👍

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

I'm a little confused on the differences in these 2 pages. I think a better title on Configuring environment variables would help. It's not about configuring them, is it?

docs/user/reference/environment-variables.rst Outdated Show resolved Hide resolved
@benjaoming
Copy link
Contributor Author

I'm a little confused on the differences in these 2 pages. I think a better title on Configuring environment variables would help. It's not about configuring them, is it?

It was a terrible title!

There are currently 3 pages in play, all of which are largely finished methinks..

  1. A how-to about defining environment variables for a project
  2. A reference to built-in environment variables that are available in the build process
  3. An explanation about accessing environment variables in the build process

The "3rd page" was called "Configuring environment variables" and the title wasn't good. I've changed it to "Understanding environment variables" and added a better introduction and some more content about sphinx-multiproject, since that AFAICT is a very solid reason to mention here.

Copy link
Member

@ericholscher ericholscher 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 a 💯 refactor. Thanks for explaining it. Definitely quite close, just a few nitpicks.

docs/user/reference/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/reference/environment-variables.rst Show resolved Hide resolved
docs/user/guides/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/guides/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/guides/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/environment-variables.rst Show resolved Hide resolved
docs/user/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/environment-variables.rst Outdated Show resolved Hide resolved
docs/user/environment-variables.rst Outdated Show resolved Hide resolved
@benjaoming benjaoming merged commit b1b877c into readthedocs:diataxis/main Feb 10, 2023
@benjaoming benjaoming deleted the diataxis/environment-variables branch February 10, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code Needed: documentation Documentation is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants