Conversation
Greptile SummaryThis PR adds a "Deployment retention" settings card to both the Functions and Sites settings pages, allowing users to automatically delete inactive deployments after a configurable number of days. The implementation follows existing patterns in the codebase — Svelte 5 runes, Confidence Score: 5/5Safe to merge; remaining finding is a minor style suggestion. The only finding is a P2 style suggestion about No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "feat: add deployment retention settings" | Re-trigger Greptile |
| label="Retention (days)" | ||
| placeholder="Enter retention" | ||
| disabled={!retentionEnabled} | ||
| required |
There was a problem hiding this comment.
required not gated on retentionEnabled
The InputNumber has required hardcoded even when disabled={!retentionEnabled}. HTML5 browser validation skips disabled inputs, but if the custom Form component performs its own required validation independently of disabled, it could show a spurious "field is required" error when a user saves with retention toggled off. Consider making required conditional:
| required | |
| required={retentionEnabled} |
The same issue exists in the sites variant (updateDeploymentRetention.svelte).
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)