Skip to content

engine: record how many times a user has opened a log in stats [ch2194]#1456

Merged
jazzdan merged 3 commits into
masterfrom
jazzdan/log_log_view
Apr 9, 2019
Merged

engine: record how many times a user has opened a log in stats [ch2194]#1456
jazzdan merged 3 commits into
masterfrom
jazzdan/log_log_view

Conversation

@jazzdan

@jazzdan jazzdan commented Apr 8, 2019

Copy link
Copy Markdown
Contributor

It's unclear to me how best to record this data. Any advice?

@jazzdan
jazzdan requested review from landism and nicks April 8, 2019 22:20
Comment thread internal/hud/hud.go Outdated
url := h.webURL
url.Path = fmt.Sprintf("/r/%s/", r.Name)
_ = browser.OpenURL(url.String())
dispatch(OpenLogAction{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there's no good reason to put this in EngineState, create actions for it, etc. It's a lot of indirection that buys you nothing. just inject analytics into the HUD and call Incr() directly.

Comment thread internal/engine/analytics_reporter.go Outdated
for index := 0; index < st.OpenLogCount; index++ {
ar.a.Incr("ui.interactions.log_open", map[string]string{})
}
st.OpenLogCount = 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also, for future reference, this assignment is a no-op. subscribers don't have write access to the EngineState. st isn't a pointer. This mutates a copy of the engine state, which is probably not what you intended.

@jazzdan
jazzdan force-pushed the jazzdan/log_log_view branch from 39d48c1 to 35abbd0 Compare April 9, 2019 14:50
@jazzdan
jazzdan requested a review from nicks April 9, 2019 14:50
Comment thread internal/hud/hud.go
}
url := h.webURL
url.Path = fmt.Sprintf("/r/%s/", r.Name)
h.a.Incr("ui.interactions.open_log", map[string]string{"is_tiltfile": strconv.FormatBool(r.Name == view.TiltfileResourceName)})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you also add this for the other OpenBrowser>

@jazzdan
jazzdan merged commit d2c43a2 into master Apr 9, 2019
@nicks
nicks deleted the jazzdan/log_log_view branch March 31, 2021 17:31
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.

2 participants