Skip to content

Conversation

@mtojek
Copy link
Member

@mtojek mtojek commented Jul 18, 2023

Related: coder/coder#6828

This PR introduces another constraint for ephemeral parameters:

  • ephemeral parameters can't be marked as required, so they must have the default property defined.

@mtojek mtojek self-assigned this Jul 18, 2023
@mtojek mtojek requested review from johnstcn and mafredri July 18, 2023 08:32
@mtojek mtojek marked this pull request as ready for review July 18, 2023 08:32
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

LGTM!

}

if !parameter.Optional && parameter.Ephemeral {
return diag.Errorf("ephemeral parameter requires the default property")
Copy link
Member

Choose a reason for hiding this comment

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

Sanity check: do parameters have an optional field or is it inferred by setting default, and is that the only way to do it? (I always stop to ask when I see the check and message say something different.)

Copy link
Member Author

Choose a reason for hiding this comment

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

do parameters have an optional field

No.

is it inferred by setting default

Yes 👍

is that the only way to do it

Yes 👍

@mtojek mtojek merged commit 1882c77 into coder:main Jul 18, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants