Skip to content

Comments

Add base_name var and use it#495

Merged
AlCutter merged 1 commit intotransparency-dev:mainfrom
AlCutter:witness_tf_base_name
Feb 17, 2026
Merged

Add base_name var and use it#495
AlCutter merged 1 commit intotransparency-dev:mainfrom
AlCutter:witness_tf_base_name

Conversation

@AlCutter
Copy link
Contributor

@AlCutter AlCutter commented Feb 17, 2026

This PR introduces a base_name variable to the witness terraform module, and uses it when naming resources it creates.

This allows for deploying multiple independent instances within the same project.

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 22.63%. Comparing base (3c58af4) to head (40c9e51).
⚠️ Report is 295 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #495       +/-   ##
===========================================
- Coverage   51.05%   22.63%   -28.42%     
===========================================
  Files          11       28       +17     
  Lines         903     1975     +1072     
===========================================
- Hits          461      447       -14     
- Misses        374     1451     +1077     
- Partials       68       77        +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mhutchinson
Copy link
Contributor

Can you explain why? I don't see anything breaking here, but it makes a few minor ergonomic regressions. The base name seems to get used directly for naming different resources, without any suffix. I don't think this causes a problem because each resource type is a different namespace, but it was nice that the service was foo-service rather than just foo. And the display name was intentional before, and now it's just the resource ID.

@AlCutter
Copy link
Contributor Author

Can you explain why? I don't see anything breaking here, but it makes a few minor ergonomic regressions. The base name seems to get used directly for naming different resources, without any suffix. I don't think this causes a problem because each resource type is a different namespace, but it was nice that the service was foo-service rather than just foo. And the display name was intentional before, and now it's just the resource ID.

The driver is to enable multiple instances to be deployed in the same project.

Spanner display name I'd have happily removed and left unset, but it's only there because it's a required field.

Dropping -service was intentional as it felt somewhat redundant having thing.myname-thing, but I can add that style back in if you feel strongly that it's better.

Copy link
Contributor

@mhutchinson mhutchinson left a comment

Choose a reason for hiding this comment

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

Sure, as long as it's intentional. I guessed this was the intent, but raised a yellow flag because base_name implied it was to be built on. Go ahead!

@AlCutter
Copy link
Contributor Author

Sure, as long as it's intentional. I guessed this was the intent, but raised a yellow flag because base_name implied it was to be built on. Go ahead!

Good call, thanks for checking!

@AlCutter AlCutter merged commit 0fcd2f0 into transparency-dev:main Feb 17, 2026
9 checks passed
@AlCutter AlCutter deleted the witness_tf_base_name branch February 17, 2026 17:10
@AlCutter AlCutter mentioned this pull request Feb 19, 2026
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