Conversation
|
@claude review |
| #if os(macOS) | ||
| products.append(.executable(name: "Timeline-macOS", targets: ["TimelineCocoa"])) | ||
| targets.append( | ||
| .executableTarget( |
There was a problem hiding this comment.
why to conditionalize it script-like instead of declarative style? This has drawback that on each platform it's going to basically be a different package file, resulting in a different Package.resolved lockfile, causing endless git diff and potential conflicts
| sources: [ | ||
| "Alerts.swift", | ||
| "Logic/FilteredAppsStorage.swift", | ||
| "Logic/PureAlignedTimer.swift", | ||
| "Logic/PureCounter.swift", | ||
| "Logic/Tracker.swift", | ||
| "Storage/Model.swift", | ||
| "Storage/Storage.swift" | ||
| ]), |
There was a problem hiding this comment.
would be nice to not have every source file to be listed in the package file
| lastAlertText = nil | ||
| } catch { | ||
| reportStorageError(error, action: "storing timeline") | ||
| if isDiskFull(error) { |
There was a problem hiding this comment.
why don't just
| if isDiskFull(error) { | |
| if error == StorageError.diskFull { |
| } catch { | ||
| reportStorageError(error, action: "storing timeline") | ||
| if isDiskFull(error) { | ||
| counter.clearAndPause() | ||
| if active { | ||
| tickAppCounter() | ||
| } | ||
| return |
There was a problem hiding this comment.
Could you explain this piece for me please? If we are stuck with the disk being full and we could not persist the data, why to clear the data?
| } catch { | ||
| reportStorageError(error, action: "storing log") | ||
| if isDiskFull(error) { | ||
| break |
There was a problem hiding this comment.
On line 110 there is the same pattern and the same check, but there is return and here is break. Could you explain the difference?
| let text = title + "|" + message | ||
| guard text != lastAlertText else { | ||
| return | ||
| } |
There was a problem hiding this comment.
I am almost sure this should be the responsibility of the target apps, but I am fine to leave it here as it is for now 👌
2 participants
No description provided.