Skip to content

feat: add coder_parameter_order to all data.coder_parameter fields #223

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

Merged
merged 4 commits into from
Apr 15, 2024

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Apr 11, 2024

Add coder_parameter_order variable to modules that have a data coder_parameter

  • aws-region
  • azure-region
  • dotfiles
  • exoscale-instance-type
  • exoscale-zone
  • gcp-region
  • git-config (has 2 parameters)

Examples

NOTE: add coder_parameter_order for git-config could be increments by one, gcp-region order should be higher to start with. So intervals by 10 is always safe

module "git-config" {
  source                = "registry.coder.com/modules/git-config/coder"
  version               = "1.0.11"
  agent_id              = coder_agent.example.id
  allow_email_change    = true
  coder_parameter_order = 10
}

module "gcp-region" {
  source                = "registry.coder.com/modules/gcp-region/coder"
  version               = "1.0.11"
  regions               = ["us", "europe"]
  coder_parameter_order = 20
}

// etc..

related to #207

@michaelbrewer michaelbrewer changed the title feat: add coder_parameter_order feat: add coder_parameter_order to all data.coder_parameter fields Apr 11, 2024
@matifali matifali requested review from mafredri and bpmct April 13, 2024 16:37
type = number
description = "The order determines the position of the 'username' template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it be feasible to just use one coder_parameter_order for all parameters? If someone wants to change these individually, then there may be something wrong with the module or how it's being used.

Why? This makes modules updates more risky. If we ever add a third parameter, it could end up anywhere when the user updates.

Perhaps we can still apply order if it's given, e.g. order = var.coder_parameter_order ? var.coder_parameter_order + 0 : null, + 1, etc.

An author can reserve as much ordered space for a module as they wish.

module "git-config" {
  source                = "registry.coder.com/modules/git-config/coder"
  version               = "1.0.11"
  agent_id              = coder_agent.example.id
  allow_email_change    = true
  coder_parameter_order = 10
}

module "gcp_region" {
  source                = "registry.coder.com/modules/gcp-region/coder"
  version               = "1.0.11"
  regions               = ["us", "europe"]
  coder_parameter_order = 20
}

To clarify this, maybe we could name it something like coder_parameter_group_order or coder_parameter_order_weight, I'm open to better ideas 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, i can keep it all the same field name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mafredri - i have updated the code and tests

@michaelbrewer michaelbrewer requested a review from mafredri April 15, 2024 15:18
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.

Nice work, thanks for the PR!

@matifali matifali merged commit a8c659a into coder:main Apr 15, 2024
2 checks passed
@michaelbrewer michaelbrewer deleted the feat/code_parameter_order branch April 15, 2024 22:07
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.

3 participants