Skip to content

Add nil filtering & improve macOS support - #43

Merged
ladd merged 2 commits into
mainfrom
lvt-buildtracer-support
Oct 24, 2025
Merged

Add nil filtering & improve macOS support#43
ladd merged 2 commits into
mainfrom
lvt-buildtracer-support

Conversation

@ladd

@ladd ladd commented Oct 23, 2025

Copy link
Copy Markdown
  • Improved macOS support. Previous implementation worked for native iOS or Catalyst builds running on macOS, but not native macOS target. (needed for BuildTracer)
  • Filter nil attribute values boxed in AnyHashable
  • Tests

@jparise
@bachand

var otlpAttributes = [OTLP.V1KeyValue]()

let keys = attributes.keys.sorted()
let keys = filteredAttributes.keys.sorted()

@jparise jparise Oct 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Given that we're only using the keys from this filtered Dictionary, I think you could also do something like this as just an expression:

let keys = compactMap { (key, value) in
    if case Optional<Any>.none = value.base {
        return nil
    }
    return key
}.sorted()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Even better: #44

@ladd
ladd merged commit cf2bdd0 into main Oct 24, 2025
4 checks passed
@ladd
ladd deleted the lvt-buildtracer-support branch October 24, 2025 20:37

@bachand bachand left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice! Thanks @ladd . I'm generally deferring to Jon's review and I also looked over the changes at a high level.

let osVersion = try XCTUnwrap(attributes["os.version"] as? String)

// Verify platform-specific OS name
#if os(macOS)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see platform-specific tests runs from CI 👍

Image

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.

3 participants