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

Discussion: provide a consistent way to override commands configuration #1265

Open
erezrokah opened this issue Sep 21, 2020 · 6 comments
Open
Labels
status: discussion type: feature code contributing to the implementation of a feature and/or user facing functionality

Comments

@erezrokah
Copy link
Contributor

When deploying a site on Netlify one can use either the UI or/and a netlify.toml file to modify deploy/build related settings.
The CLI allows overriding some of those via flags (like functions/publish directory).
Other CLI behaviour is controlled via env variables:

let browser = process.env.BROWSER

I believe we should be consistent in the CLI with how we treat flags, env variables and which part of the configuration can be modified using those.

I suggest using the following conventions:

  1. Flags have highest priority, then env variables then netlify.toml and UI (the last two are handled in Netlify conifg package).
  2. Prefix env variables with NETLIFY_ except ones that are meant to be shared by tools (like BROWSER,HTTP_PROXY)
  3. Each env variable should have a corresponding flag and vice versa with a naming convention so users don't need to remember how to switch between them (unless we drop env variables completely).

As for overriding netlify.toml/UI settings I personally believe we should provide good defaults and make everything configurable via flags, but that might become cumbersome.

Thoughts?

@erezrokah erezrokah added type: feature code contributing to the implementation of a feature and/or user facing functionality status: discussion labels Sep 21, 2020
@ehmicky
Copy link
Contributor

ehmicky commented Sep 21, 2020

This all sounds good to me (including priority order and variables prefixing), with two small points.

Each env variable should have a corresponding flag and vice versa

Providing CLI flags as an alternative to each environment variable sounds good. However, in the other direction, would providing more environment variables (to reflect each CLI flag) encourage users to use those instead of CLI flags? Is this something we want?

As for overriding netlify.toml/UI settings I personally believe we should provide good defaults and make everything configurable via flags, but that might become cumbersome.

I agree we should allow users to override build settings using CLI flags (although I don't have a strong opinion either way). Computing build settings requires retrieving and merging UI settings, normalizing them (e.g. there are several possible cases, paths can optionally start with /, and so on), validating them, etc. That logic is currently performed inside @netlify/config.

cli/src/utils/command.js

Lines 78 to 81 in 9ce3a91

// Find and resolve the Netlify configuration
async getConfig(cwd, state, token, argv) {
try {
return await resolveConfig({

@netlify/config has an --inlineConfig CLI flag which allows overriding both the netlify.toml and the UI build settings, e.g. --inlineConfig.build.functions=my-functions. This could be used as a general solution to overriding build settings from the CLI.

Not every CLI command requires all build settings, so we might want to consider the trade-offs of generalizing this solution vs implementing it for each CLI command.

@erezrokah
Copy link
Contributor Author

Providing CLI flags as an alternative to each environment variable sounds good. However, in the other direction, would providing more environment variables (to reflect each CLI flag) encourage users to use those instead of CLI flags? Is this something we want?

In some environments it's easier/common to use env variables (e.g. CI) and other flags (local dev).
I'm mostly interested in avoiding the strain of trying to remember which option has a flag and which one supports an env as a fallback.

@ehmicky
Copy link
Contributor

ehmicky commented Sep 21, 2020

This makes sense. Providing a general solution might also help simplify the code by abstracting it to a common utility.

@erezrokah
Copy link
Contributor Author

Adding to the confusion, users can also use the [dev] block and context.dev for various configurations.

@KyleBlankRollins
Copy link
Contributor

We got some customer feedback on the file-based configuration docs page about [context.dev.environment] not being documented.

I found the documentation for Netlify Dev context via a support ticket answer: https://answers.netlify.com/t/context-name-for-netlify-dev/533/6 Would be nice if that was included here.

They're correct that we don't have it in the official docs, but it is in the support forums. Is this something we want to document in the Netlify Dev section, or do we want to wait until we have what this issue is proposing (consistent way to override)? And in general, do we want to address gaps like this, or wait until we have a consistent override strategy?

@erezrokah
Copy link
Contributor Author

Thanks @KyleBlankRollins, I think it makes sense to document context.dev.environment per this issue as that's the expected behavior.

And in general, do we want to address gaps like this, or wait until we have a consistent override strategy?

I'm not sure, as some gaps might be related to CLI bugs and could be considered as features unintentionally. So I would take this one case as a time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: discussion type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

No branches or pull requests

3 participants