-
Notifications
You must be signed in to change notification settings - Fork 8
Description
Summary
Standardize function signatures so that Settings (or &Settings) is always the first parameter when present. Currently the position varies across the codebase.
Current State
| Function | Settings Position | File |
|---|---|---|
enforce_basic_auth(settings, req) |
First ✓ | crates/common/src/auth.rs:9 |
generate_synthetic_id(settings, req) |
First ✓ | crates/common/src/synthetic.rs:30 |
encode_url(settings, plaintext_url) |
First ✓ | crates/common/src/http_util.rs:47 |
decode_url(settings, token) |
First ✓ | crates/common/src/http_util.rs:74 |
proxy_request(settings, req, config) |
First ✓ | crates/common/src/proxy.rs:303 |
rewrite_creative_html(body, settings) |
Last ✗ | crates/common/src/creative.rs |
serve_static_with_etag(body, req, content_type) |
Not present | crates/common/src/http_util.rs:11 |
Target State
All functions that accept Settings should have it as the first parameter:
// Before
fn rewrite_creative_html(body: &str, settings: &Settings) -> Result<String, Error>
// After
fn rewrite_creative_html(settings: &Settings, body: &str) -> Result<String, Error>Rationale
- Consistency - Easier to read and understand function signatures
- Convention - Follows common Rust patterns where context/config comes first
- Discoverability - When scanning code, settings parameter is always in the same place
Changes Required
- Identify all functions with
Settingsparameter not in first position - Update function signatures to move
Settingsto first position - Update all call sites to match new signatures
- Run tests to ensure no regressions
Files to audit
crates/common/src/creative.rscrates/common/src/html_processor.rscrates/common/src/integrations/*.rs
Testing
cargo build- Ensure compilation succeedscargo test- Ensure all tests passcargo clippy- No new warnings
Complexity
Low - Mechanical refactoring with IDE support
Labels
good first issuetech-debtrefactoring