Skip to content

fix(config): drop meaningless DefaultSettings UpdatedAt timestamps - #1056

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/config-env-warnings
Jun 7, 2026
Merged

fix(config): drop meaningless DefaultSettings UpdatedAt timestamps#1056
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/config-env-warnings

Conversation

@cristim

@cristim cristim commented Jun 7, 2026

Copy link
Copy Markdown
Member

Summary

Static DefaultSettings entries in internal/config/defaults.go stamped
UpdatedAt: time.Now() at package init -- a meaningless timestamp that could
produce false-positive "recently changed" signals versus DB-persisted user
settings.

  • Replace all 28 time.Now() calls with time.Time{} (zero value).
  • Add a comment documenting the zero-UpdatedAt contract.
  • Rename and rewrite TestDefaultSettings_UpdatedAtSet to
    TestDefaultSettings_UpdatedAtIsZero, asserting zero value instead.
  • Remove the now-unused "time" import from defaults_test.go.

Closes #1051

Note on 04-M1 overlap with #1040

Finding 04-M1 (warn on malformed env int/float values) is already addressed
by PR #1040
(fix/server-transport-config): getEnvInt, getEnvFloat, and
getTaskTimeout all log WARNING: on parse failure in that branch. This PR
covers only the residual 04-N2 item (see docs/code-review/open-questions/new-config-warn.md).

Verification

go build ./internal/config/...   # clean
go vet ./internal/config/...     # clean
go test ./internal/config/...    # 560 passed
go build ./...                   # clean

…ero time

Static default settings are never "updated" by a user; stamping UpdatedAt
with time.Now() at init was misleading and could produce false-positive
"recently changed" signals. Replace with time.Time{} (zero value) to clearly
distinguish compile-time defaults from DB-persisted user updates.

Update TestDefaultSettings_UpdatedAtSet to assert the zero value instead of
asserting non-zero, and remove the now-unused "time" import.

Closes #1051
@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/few Limited audience effort/xs Trivial / one-liner type/bug Defect labels Jun 7, 2026
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 17 minutes and 32 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc363e0f-d192-43c9-9b23-da31953350f8

📥 Commits

Reviewing files that changed from the base of the PR and between cbdc4be and cf44a75.

📒 Files selected for processing (2)
  • internal/config/defaults.go
  • internal/config/defaults_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/config-env-warnings

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim

cristim commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

Changes summary

fix(config): drop meaningless DefaultSettings UpdatedAt timestamps (finding 04-N2)

  • internal/config/defaults.go: replaced 28 time.Now() calls with time.Time{} (zero value); added doc comment explaining the zero-UpdatedAt contract.
  • internal/config/defaults_test.go: renamed TestDefaultSettings_UpdatedAtSet to TestDefaultSettings_UpdatedAtIsZero; updated assertion to require zero value; removed unused "time" import.

04-M1 disposition: confirmed subsumed by #1040 -- getEnvInt/getEnvFloat/getTaskTimeout in that branch already log WARNING: on parse failure. Decision logged to docs/code-review/open-questions/new-config-warn.md.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

@cristim Sure, I'll kick off a review of this PR now.


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim merged commit 7bdf5fd into feat/multicloud-web-frontend Jun 7, 2026
4 checks passed
@cristim
cristim deleted the fix/config-env-warnings branch July 27, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant