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

Bug 1693486 - New event extra properties #1567

Closed
wants to merge 2 commits into from
Closed

Conversation

badboy
Copy link
Member

@badboy badboy commented Mar 24, 2021

So far this is an early draft that implements the new calling convention in Kotlin and Swift.
Stuff to do before this is ready though.

  • Implement it in glean-parser
  • Decide on the name of the new interface/protocol
  • Prototype the C++/JavaScript API for FOG
  • Python implementation (might be split out into a followup)
  • Kotlin
    • Connection between the enum <-> interface, so the EventMetricType is generic over the right thing (necessary to avoid using the some extras on the wrong event)
  • Swift
    • Connection between the enum <-> protocol, so the EventMetricType is generic over the right thing (necessary to avoid using the some extras on the wrong event)

@badboy
Copy link
Member Author

badboy commented Mar 24, 2021

Only just now realized we're also changing the payload format. That's ... more work

@badboy
Copy link
Member Author

badboy commented Mar 24, 2021

Turns out it's a bit more complicated than initially sketched out.
We also want to change the payload, which means we can't do the $type -> string conversion on the language binding side, but need to pass in the right values.
So we need to separate out the two APIs to handle them differently.

@badboy badboy added the blocked Blocked pull requests and issues label Mar 26, 2021
@badboy
Copy link
Member Author

badboy commented Mar 26, 2021

Blocked by the discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1693486

@badboy
Copy link
Member Author

badboy commented Apr 29, 2021

Closing this in favor of only changing the user-facing API (which will probably include this code, but let's start with a fresh PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked pull requests and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant