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

Update build environment logic #477

Closed
wants to merge 3 commits into from

Conversation

tlindsay42
Copy link
Contributor

@tlindsay42 tlindsay42 commented Jan 3, 2022

Closes #453
Relates to #457

  • Adds GetEnvironmentName and IsProduction partial functions, which improve DRY
  • Add prioritization for determining environment name, which favors the existing methods: the HUGO_ENV environment variable and site config env theme parameter
  • Adds useHugoEnv theme parameter to use the value of .Hugo.Environment for the build environment name

* Adds GetEnvironmentName and IsProduction partial functions, which
  improve DRY
* Add prioritization for determining environment name, which favors the
  existing methods: the `HUGO_ENV` environment variable and site config
  `env` theme parameter
* Adds `useHugoEnv` theme parameter to use the value of
  `.Hugo.Environment` for the build environment name
@tlindsay42
Copy link
Contributor Author

Ready for review @regisphilibert

@budparr budparr requested a review from regisphilibert January 4, 2022 15:46
@regisphilibert
Copy link
Member

regisphilibert commented Jan 10, 2022

Thanks @tlindsay42 for all you work this past weeks 🙏 . I feel like this one deserves some thinking though. JFY, we're not ignoring it but might ponder it for some time.

@tlindsay42
Copy link
Contributor Author

tlindsay42 commented Jan 10, 2022

Sounds good. My primary driver for this feature was that I chose to implement my site with the env-specific config files (eg: ./config/_default/config.toml and ./config/production/config.toml) because I wanted to define a few env-specific behaviors, such as minification. Given this, I was confused at first when parts of the site ran in production mode when I ran hugo server --environment production, but other features didn't.

To test production mode with this type of implementation, customers have to potentially set both the environment variable and the argument, which isn't that big of a deal, but imo it's annoying and not obvious.

Since the feature is opt-in only, existing customers that aren't expecting this won't be impacted (like we previously discussed).

I'm open to suggestions if changes are desired.

I hope you decide to merge it, but understand if the decision is otherwise. Thanks!

@rgson
Copy link

rgson commented May 29, 2022

+1. I just started using this theme and was confused by the --environment option not being sufficient.

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.

Support for .Hugo.Environment variable
3 participants