DX improvements and more info in logs #93
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This way, we avoid other `os_log` calls getting in between `os_log` and `dump` and breaking the association between the two which makes the logs harder to understand.
mokagio
commented
Nov 28, 2024
Comment on lines
+6
to
+7
| excluded: | ||
| - .build |
There was a problem hiding this comment.
This ought to go in the the remote config, too.
mokagio
commented
Nov 28, 2024
Sources/Track.swift
Outdated
| private func os_log_sending_event(_ event: Event, log: OSLog = .tracker, type: OSLogType = .debug) { | ||
| var eventDump = DumpOutput() | ||
| dump(event.toDict(), to: &eventDump) | ||
| os_log("Sending an event from Track:\n%@", log: log, type: type, eventDump.content) |
There was a problem hiding this comment.
Note to self: Revisit the os_log implementation and consider using modern Logger, see Create a Log Object to Organize Messages, which supports string interpolation.
mokagio
commented
Nov 28, 2024
| -scheme ParselyDemo \ | ||
| -sdk iphonesimulator \ | ||
| -destination 'platform=iOS Simulator,name=$(SIMULATOR_NAME),OS=$(SIMULATOR_OS)' \ | ||
| | xcpretty |
There was a problem hiding this comment.
I verified that this new syntax with the set -o pipefail in the Make task is still valid in CI:
See #95
crazytonyli
reviewed
Nov 28, 2024
| mutating func write(_ string: String) { | ||
| content.append(string) | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think this type is needed.
$ swift repl
Welcome to Apple Swift version 6.0.2 (swiftlang-6.0.2.1.2 clang-1600.0.26.4).
Type :help for assistance.
1> var content = ""
content: String = ""
2> dump([1:2], to: &content)
$R0: [Int : Int] = 1 key/value pair {
[0] = {
key = 1
value = 2
}
}
3> print(content)
▿ 1 key/value pair
▿ (2 elements)
- key: 1
- value: 2
There was a problem hiding this comment.
Correct, 06c8b36 Thanks!
twstokes
reviewed
Jan 3, 2025
|
|
||
| # Convenience to open the lib and demo project with Xcode, given it's not in the root. | ||
| open: | ||
| open -a $(XCODE_PATH) ./Demo/ParselyDemo.xcodeproj |
There was a problem hiding this comment.
Is there an advantage to using this method over xed?
twstokes
reviewed
Jan 3, 2025
| @@ -82,3 +81,10 @@ class Track { | |||
| videoManager.sendHeartbeats() | |||
| } | |||
| } | |||
|
|
|||
| /// Utitlity to log sending event with a dump of the event. | |||
There was a problem hiding this comment.
Suggested change
| /// Utitlity to log sending event with a dump of the event. | |
| /// Utility to log sending event with a dump of the event. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In trying to help a customer with a question they had, p1731493061054529-slack-C0533SEJ82H, I wanted to look into the logs from the demo app.
In the meantime I noticed a few things the project could use:
Makefileto use as the single source of truth for the command to test the SDK.editorconfig, in particular now that Xcode supports it, https://x.com/mokagio/status/1839809925570265100dumpoutput.