Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DX improvements and more info in logs #93

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Apply to all files
[*]
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true

# Ruby specific rules
[{*.rb,Fastfile,Gemfile}]
indent_style = space
indent_size = 2

[*.{swift,h,m}]
indent_style = space
indent_size = 4
9 changes: 1 addition & 8 deletions .github/workflows/ios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,4 @@ jobs:
with:
xcode-version: ${{ matrix.xcode }}
- name: Build and Test
run: |
xcodebuild test \
-project Demo/ParselyDemo.xcodeproj \
-scheme ParselyDemo \
-sdk iphonesimulator \
-destination 'platform=iOS Simulator,name=iPhone 14,OS=latest' \
| xcpretty \
&& exit ${PIPESTATUS[0]}
run: make test
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ xcuserdata/*

# Ruby tooling
vendor/bundle


# SwiftLint Remote Config Cache
.swiftlint/RemoteConfigCache
7 changes: 7 additions & 0 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
swiftlint_version: 0.57.0

parent_config: https://raw.githubusercontent.com/Automattic/swiftlint-config/b497131f8d0fddbf3b23278cfc4ef8d86c9bcb20/.swiftlint.yml
remote_timeout: 10.0

excluded:
- .build
Comment on lines +6 to +7
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ought to go in the the remote config, too.

22 changes: 22 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# TODO: Use newer Sim. Sticking with it at the moment because we know it works in the existing GitHub action setup.
SIMULATOR_NAME ?= iPhone 14
SIMULATOR_OS ?= latest
XCODE_PATH ?= /Applications/Xcode.app

# 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an advantage to using this method over xed?


# TODO: Move off xcpretty to xcbeautify. Sticking with it at the moment because we know it works in the existing GitHub action setup.
test:
set -o pipefail \
&& xcodebuild test \
-project Demo/ParselyDemo.xcodeproj \
-scheme ParselyDemo \
-sdk iphonesimulator \
-destination 'platform=iOS Simulator,name=$(SIMULATOR_NAME),OS=$(SIMULATOR_OS)' \
| xcpretty
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that this new syntax with the set -o pipefail in the Make task is still valid in CI:

image

See #95


# TODO: Add automation to set up SwiftLint
format:
swiftlint --fix --format
10 changes: 8 additions & 2 deletions Sources/Track.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ class Track {
parselyTracker.startFlushTimer();

pixel.beacon(event: event)
os_log("Sending an event from Track", log: OSLog.tracker, type:.debug)
dump(event.toDict())
os_log_sending_event(event)
}

func pageview(url: String, urlref: String = "", metadata: ParselyMetadata?, extra_data: Dictionary<String, Any>?, idsite: String) {
Expand Down Expand Up @@ -82,3 +81,10 @@ class Track {
videoManager.sendHeartbeats()
}
}

/// Utitlity to log sending event with a dump of the event.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Utitlity to log sending event with a dump of the event.
/// Utility to log sending event with a dump of the event.

private func os_log_sending_event(_ event: Event, log: OSLog = .tracker, type: OSLogType = .debug) {
var eventDump = ""
dump(event.toDict(), to: &eventDump)
os_log("Sending an event from Track:\n%@", log: log, type: type, eventDump)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@mokagio mokagio Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, 06c8b36 Thanks!

Loading