Skip to content

Rename LogLevel.warn to warning and add LogLevel.silent#111

Merged
tomas-martins-crossmint merged 8 commits into
mainfrom
tomas/wal-9980-logging-improvements
Jun 29, 2026
Merged

Rename LogLevel.warn to warning and add LogLevel.silent#111
tomas-martins-crossmint merged 8 commits into
mainfrom
tomas/wal-9980-logging-improvements

Conversation

@tomas-martins-crossmint

@tomas-martins-crossmint tomas-martins-crossmint commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

LogLevel had a warn case instead of warning, and no way to silence console output entirely.

Note: warning diverges from other SDKs, which use warn. This is intentional — it matches Kotlin and the EDD.

Changes

  • Renames LogLevel.warn to LogLevel.warning across the entire Logger stack: the protocol, both providers (OSLog and DataDog), and every call site in the SDK
  • Adds LogLevel.silent — sits above .error in raw value order, suppressing all console output. DataDog is unaffected and always receives logs regardless of the configured level
  • Moves the log level filtering from Logger into OSLoggerProvider so that remote logging is never silenced by the level config, matching the behavior of the Kotlin and TS SDKs
  • Adds a LoggerTests target with tests covering the new filtering behavior: that remote providers always receive logs, and that console providers respect the configured level

@tomas-martins-crossmint tomas-martins-crossmint marked this pull request as draft June 24, 2026 21:07
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Sources/CrossmintClient/Configuration.swift:7-15
**Documentation example references non-existent API**

The doc comment shows `CrossmintSDK.configure(with: Configuration(...))`, but no such overload exists in `CrossmintSDK.swift` — only `configure(apiKey:logLevel:)` is available. Any user who copies this example will get a compile error. Since `Configuration` is exported as `public`, this will appear in Xcode quick-help immediately after merging. The doc comment should either be removed/updated to reflect the current API, or the corresponding `configure(with:)` overload should be added in this PR.

### Issue 2 of 2
Sources/Logger/Remote/DataDogLoggerProvider.swift:284-285
**Dead `.none` case silently falls back to `"info"`**

`write(level:)` is private and is only ever called from the four `nonisolated func debug/info/warning/error` methods, so `.none` can never actually reach `mapLevelToStatus`. However, the exhaustive switch is still required, and mapping `.none` to `"info"` is a slightly surprising default. A more defensive choice that makes intent obvious is to use `"none"` so that any future code path that accidentally passes `.none` here is immediately visible.

```suggestion
        case .none:
            return "none"
```

Reviews (1): Last reviewed commit: "Remove Configuration struct — out of sco..." | Re-trigger Greptile

Comment thread Sources/Logger/Remote/DataDogLoggerProvider.swift Outdated
@tomas-martins-crossmint tomas-martins-crossmint marked this pull request as ready for review June 24, 2026 22:23
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Reviews (2): Last reviewed commit: "Remove DocC comment from LogLevel.none" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Reviews (3): Last reviewed commit: "Move log level filtering to OSLoggerProv..." | Re-trigger Greptile

@tomas-martins-crossmint tomas-martins-crossmint changed the title Rename LogLevel.warn to warning and add LogLevel.none Rename LogLevel.warn to warning and add LogLevel.silent Jun 26, 2026
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Reviews (4): Last reviewed commit: "Rename LogLevel.none to silent" | Re-trigger Greptile

@tomas-martins-crossmint tomas-martins-crossmint force-pushed the tomas/wal-9980-logging-improvements branch from 5ea2508 to 0ff88eb Compare June 26, 2026 21:07
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Reviews (5): Last reviewed commit: "Rename warn to warning in CrossmintTEE (..." | Re-trigger Greptile

@tomas-martins-crossmint tomas-martins-crossmint force-pushed the tomas/wal-9980-logging-improvements branch from 0ff88eb to 2baa3fd Compare June 29, 2026 14:18
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Reviews (6): Last reviewed commit: "Rename warn to warning in CrossmintTEE (..." | Re-trigger Greptile

@tomas-martins-crossmint tomas-martins-crossmint merged commit a1cbb7a into main Jun 29, 2026
4 of 5 checks passed
@tomas-martins-crossmint tomas-martins-crossmint deleted the tomas/wal-9980-logging-improvements branch June 29, 2026 15:36
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.

2 participants