-
Notifications
You must be signed in to change notification settings - Fork 0
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
Updated funnel schemas #4
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
syntax = "proto3"; | ||
|
||
package buda.entities; | ||
|
||
import "buda/entities/uuid.proto"; | ||
import "buda/entities/link.proto"; | ||
import "google/protobuf/timestamp.proto"; | ||
|
||
message FunnelEvent { | ||
UUID id = 1; | ||
UUID funnel_id = 2; | ||
UUID funnel_step_id = 3; | ||
UUID user_id = 4; | ||
google.protobuf.Timestamp created_at = 5; | ||
map<string, string> tags = 6; | ||
bool funnel_end = 16; | ||
repeated Link links = 17; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
syntax = "proto3"; | ||
|
||
package buda.entities; | ||
|
||
import "buda/entities/uuid.proto"; | ||
|
||
message Link { | ||
string target = 1; | ||
UUID target_id = 2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
syntax = "proto3"; | ||
|
||
package buda.entities; | ||
|
||
message UUID { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this approach! 😄 I did a bit of 🔍 and seems there's an open issue to represent UUIDs in protobufs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kinda a more general question but, how do you think we could name messages like Looking at their go There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good one @davidgasquez! To be honest, I glossed over the naming a bit and didn't give it much thought. Happy to use |
||
string id = 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if you see Link being used in other entities? Thinking it could be moved inside
funnel_event.proto
if not! 😊There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidgasquez Yeah, was kind of on the fence on that, but I thought 'maybe' it could be used by other entities. Reflecting on it now, I'm thinking perhaps I should stick to the current use case!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidgasquez still can't make up my mind on this one :D Maybe I could keep things as is (still want to validate if the whole Links idea is worth it) and come back to it later (before we go 'live')