Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: remove dynamic-parameters experiment#18290

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
jaaydenh merged 17 commits intomainfromjaaydenh/remove-dynamic-params-experiment
Jun 12, 2025

Conversation

jaaydenh
Copy link
Contributor

No description provided.

@jaaydenhjaaydenh self-assigned thisJun 9, 2025
jaaydenhand others added5 commitsJune 9, 2025 14:47
- Regenerated TypeScript types to remove dynamic-parameters experiment- Updated useDynamicParametersOptOut hook to always return optedOut: true  since the experiment was removed- This fixes test failures related to dynamic parameters functionalityCo-authored-by: jaaydenh <1858163+jaaydenh@users.noreply.github.com>
@blink-soblink.so
Copy link
Contributor

Fix for Test Failures

I've identified and fixed the test failures in this PR. The issues were:

1.TestPatchTemplateMeta/ClassicParameterFlow

The test was expectingUseClassicParameterFlow to befalse by default, but the new migration (000334_dynamic_parameters_opt_out.up.sql) sets it totrue. I updated the test assertion to expect the correct default value.

2.TestDynamicParametersWithTerraformValues/RebuildParameters

The test was expecting dynamic parameters to work, but withuse_classic_parameter_flow now defaulting totrue, dynamic parameters are disabled by default. I updated the test setup to explicitly enable dynamic parameters by settingUseClassicParameterFlow tofalse for the test templates.

Changes Made:

  • Updatedcoderd/templates_test.go line 1550 to expecttrue instead offalse
  • Updatedcoderd/parameters_test.go to explicitly disable classic parameter flow in tests that require dynamic parameters
  • Fixed variable shadowing issues introduced by the changes

The fix has been pushed to a separate branch:https://github.com/coder/coder/tree/pr-18290

These changes ensure the tests align with the new default behavior where templates opt out of dynamic parameters by default.

@jaaydenhjaaydenh requested a review fromEmyrkJune 11, 2025 09:39
@@ -249,6 +249,7 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
Value: "GO",
},
}
request.EnableDynamicParameters = true
Copy link
Member

Choose a reason for hiding this comment

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

We should really set the template to dynamic params, because we are going to drop this field soon.

Copy link
Member

Choose a reason for hiding this comment

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

I changed it here:414801d

Comment on lines 1044 to 1067
func (b *Builder) usingDynamicParameters() bool {
vals, err := b.getTemplateTerraformValues()
if b.dynamicParametersEnabled != nil {
return *b.dynamicParametersEnabled
}

tpl, err := b.getTemplate()
if err != nil {
return false // Let another part of the code get this error
}
if tpl.UseClassicParameterFlow {
return false
}

if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) {
vals, err := b.getTemplateTerraformValues()
if err != nil {
return false
}

ifb.dynamicParametersEnabled != nil {
return*b.dynamicParametersEnabled
if!ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) {
returnfalse
}

tpl, err := b.getTemplate()
if err != nil {
return false // Let another part of the code get this error
}
return !tpl.UseClassicParameterFlow
return true
}
Copy link
Member

Choose a reason for hiding this comment

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

👍

@jaaydenhjaaydenh marked this pull request as ready for reviewJune 11, 2025 20:24
@codercoder deleted a comment fromblink-sobotJun 12, 2025
@jaaydenhjaaydenh requested a review fromEmyrkJune 12, 2025 12:00
@jaaydenhjaaydenh merged commitf126931 intomainJun 12, 2025
86 of 91 checks passed
@jaaydenhjaaydenh deleted the jaaydenh/remove-dynamic-params-experiment branchJune 12, 2025 16:15
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJun 12, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@EmyrkEmyrkEmyrk approved these changes

Assignees

@jaaydenhjaaydenh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@jaaydenh@Emyrk

[8]ページ先頭

©2009-2025 Movatter.jp