Skip to content

refactor: drop viper #761 BindEnv workaround#412

Merged
fank merged 1 commit into
mainfrom
refactor/drop-viper-bindenv-workaround
May 1, 2026
Merged

refactor: drop viper #761 BindEnv workaround#412
fank merged 1 commit into
mainfrom
refactor/drop-viper-bindenv-workaround

Conversation

@fank
Copy link
Copy Markdown
Member

@fank fank commented May 1, 2026

Summary

  • viper #761 was closed in Dec 2023, and viper v1.21.0 (already in go.mod) resolves env vars via AutomaticEnv() during Unmarshal as long as the key has a SetDefault. The explicit BindEnv loop is no longer needed.
  • Drop the 35-entry envKeys slice + loop, add empty defaults for the six previously-unbound keys (secret, customize.websiteURL, customize.websiteLogo, customize.disableKillCount, customize.headerTitle, customize.headerSubtitle) so they continue to resolve from env.
  • Behavior change: the removed loop accidentally bound env names without the OCAP_ prefix (e.g. CONVERSION_ENABLED). Only the documented OCAP_-prefixed names are accepted now, matching SetEnvPrefix("ocap") and CLAUDE.md. The affected nested env vars with underscore test was updated accordingly.

Test plan

  • go test ./internal/server/... passes
  • go build ./... clean
  • Manual: confirm OCAP_SECRET, OCAP_CUSTOMIZE_WEBSITEURL, OCAP_CONVERSION_ENABLED, etc. still take effect at runtime

viper v1.21.0 resolves env vars via AutomaticEnv() during Unmarshal as
long as a SetDefault exists for the key, making the explicit BindEnv
loop redundant. Add empty defaults for the six previously-unbound keys
(secret + customize.* overrides) so they continue to resolve.

The removed loop also bound env names without the OCAP_ prefix as a
side effect; update the affected test to use the documented OCAP_
prefix.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/OCAP2/web/internal/server 94.06% (+0.09%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/OCAP2/web/internal/server/setting.go 92.86% (+1.55%) 70 (+1) 65 (+2) 5 (-1) 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/OCAP2/web/internal/server/setting_test.go

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several new default configuration settings for the server, including a secret key and various customization options such as website URL, logo, and header titles. Additionally, it removes a manual environment variable binding workaround in internal/server/setting.go and updates the test suite to use the OCAP_ prefix for environment variables. I have no feedback to provide.

@fank fank merged commit f6069eb into main May 1, 2026
2 checks passed
@fank fank deleted the refactor/drop-viper-bindenv-workaround branch May 1, 2026 18:42
smitt14ua pushed a commit to smitt14ua/OCAP2-web that referenced this pull request May 1, 2026
viper v1.21.0 resolves env vars via AutomaticEnv() during Unmarshal as
long as a SetDefault exists for the key, making the explicit BindEnv
loop redundant. Add empty defaults for the six previously-unbound keys
(secret + customize.* overrides) so they continue to resolve.

The removed loop also bound env names without the OCAP_ prefix as a
side effect; update the affected test to use the documented OCAP_
prefix.
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.

1 participant