refactor(vault-cli): use default = null instead of default = "" for vault_token#889
refactor(vault-cli): use default = null instead of default = "" for vault_token#889luyi66243-maker wants to merge 4 commits into
Conversation
…ault_token Replace empty string default with null for the vault_token variable, aligning with Terraform best practices and the existing vault_namespace variable pattern in the same module. This is part of the broader initiative in coder#755. Changes: - vault_token default: "" → null - templatefile: guard with null check to pass empty string to shell - coder_env count: check against null instead of empty string All existing tests pass without modification.
|
Looks Like a quick and easy fix. All we need is the version to be bumped in the module for this if you could run the version bump scripts we should be good here. Thank you for your first Contribution on our repo! |
Run the repository version bump script for a patch release so PR coder#889 includes the required module version update. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the quick review! I ran the version bump script for this module and pushed the update.\n\n- Module bumped: \n- Version: → \n\nCould you please take another look? |
|
Correction (formatting issue in my previous comment): Thanks for the quick review! I ran the version bump script for this module and pushed the update.
Could you please take another look? |
Can you run |
Co-authored-by: Cursor <cursoragent@cursor.com>
Run the repository formatting command and apply the Terraform snippet alignment changes in the vault-cli README. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Ran .agents/skills/coder-modules/SKILL.md 84ms (unchanged) |
|
Correction to my previous comment (formatting mishap): I have run the repository formatting command and pushed the resulting change for the vault-cli README formatting. Latest commit:
Could you please take another look and confirm CI on this latest commit? |
|
Could a maintainer please re-run CI checks for the latest commit ()?\n\nI applied the requested formatting update and pushed it, but checks have not triggered yet on my side. Thanks! |
|
Could a maintainer please re-run CI checks for the latest commit 6628761?\n\nI applied the requested formatting update and pushed it, but checks have not triggered yet on my side. Thanks! |
|
Quick consolidated update:\n\n- Version bump applied for coder/vault-cli (v1.1.1 -> v1.1.2)\n- Requested formatting applied (.agents/skills/coder-modules/SKILL.md 87ms (unchanged) |
|
Maintainer follow-up: please ignore my malformed comments caused by shell formatting issues.\n\nCurrent status is:\n- version bump is applied for coder/vault-cli from 1.1.1 to 1.1.2\n- formatting update is applied and pushed in commit 6628761\n- checks are not triggering for the latest commit from my fork\n\nCould a maintainer please trigger or re-run CI checks for PR 889? Thank you. |
matifali
left a comment
There was a problem hiding this comment.
@luyi66243-maker You don't need to paste the whole output in a comment. And please keep your comments GFM formatted. Looks like an AI is doing it on your behalf without disclosure.
Please check https://coder.com/docs/about/contributing/AI_CONTRIBUTING and follow these.
Summary
Replace
default = ""withdefault = nullfor thevault_tokenvariable in the vault-cli module, addressing part of #755.Using
nullinstead of""is better Terraform practice — it semantically means "not provided" rather than "provided but empty". This also alignsvault_tokenwith the existingvault_namespacevariable in the same module, which already usesdefault = null.Changes
vault_tokendefault:""→nulltemplatefilecall: added null guard (var.vault_token != null ? var.vault_token : "") to safely pass to shell templatecoder_env.vault_tokencount: changed condition from!= ""to!= nullTesting
All 10 existing Terraform tests pass without modification:
Notes
This is a scoped first step toward #755. If the approach is acceptable, I'm happy to submit follow-up PRs for other modules.