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

Allow arbitrary environment names #539

Merged
merged 3 commits into from
Dec 8, 2023

Conversation

NSeydoux
Copy link
Contributor

@NSeydoux NSeydoux commented Dec 8, 2023

We shouldn't use environment names as meaningful flags to enable or disable tests, but rather use specific feature flags. Thereforce, the environment name becomes only an informative property, and doesn't need any predefined values.

We shouldn't use environment names as meaningful flags to enable or
disable tests, but rather use specific feature flags. Thereforce, the
environment name becomes only an informative property, and doesn't need
any predefined values.
@NSeydoux NSeydoux requested a review from a team as a code owner December 8, 2023 12:17
Copy link
Contributor

@jeswr jeswr left a comment

Choose a reason for hiding this comment

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

If we are removing this gate, could you make any check that relies on environment variables vase insensitive.

I find this is useful for helping me catch this like using a lower-case s for Podspaces in my configuration.

@NSeydoux
Copy link
Contributor Author

NSeydoux commented Dec 8, 2023

I'm not sure this will be possible in at least one critical place: environment names are mostly useful in the CI matrices, where they are used to bind to environments, and that comparison is strict. If the environment names mismatch, then the environment configuration (i.e. the secrets) will not be loaded, the the test will necessarily fail.

If that's valuable, we can keep the current restrictions, rotate dev-next to dev-2-1, and also add an escape hatch, like "custom", to specify when running this in CI against a freshly build version of the server, as opposed to a stable released one.

@jeswr
Copy link
Contributor

jeswr commented Dec 8, 2023

I would be in favour of a "custom" option.

Change the supported environment name from "dev-next" to "dev-2-1", and
also add "Custom" when no fixed version correctly applies.
@jeswr jeswr merged commit 187906e into main Dec 8, 2023
5 checks passed
@jeswr jeswr deleted the chore/SDK-3257/allow-unknown-environment branch December 8, 2023 14:37
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.

2 participants