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

Add Analytics Tracking #58

Merged
merged 1 commit into from May 2, 2017
Merged

Add Analytics Tracking #58

merged 1 commit into from May 2, 2017

Conversation

ghost
Copy link

@ghost ghost commented Apr 14, 2017

adds google analytics tracking using the async tracking snippet

closes #57

https://developers.google.com/analytics/devguides/collection/analyticsjs/

adds google analytics tracking using the async tracking snippet

closes #57
@ghost ghost changed the title feat(layouts): add google analytics Add Analytics Tracking Apr 14, 2017
@eduardoboucas
Copy link
Member

Great idea! Only one tiny detail: we use camel case for variables in profiles, but you're adding a snake case here. Should we consolidate here?

@ghost
Copy link
Author

ghost commented Apr 15, 2017

Force of habit, and a bit more in line with what SEO Tag Gem and other core friends of Jekyll are doing. But consistency is key. Updating.

@ghost
Copy link
Author

ghost commented Apr 15, 2017

Actually. I'm going to renege my last comment. We should standardize on snake. I'm doing the research now.

@ghost
Copy link
Author

ghost commented Apr 15, 2017

Here it is: https://github.com/bbatsov/ruby-style-guide#naming

Use snake_case for symbols, methods and variables.

Because Jekyll was born out of the Rails community, and enjoys many of the same Rails community tooling (e.g. Sprockets, etc.) we should standardize on lower snake.

Additionally, the most popular Jekyll themes use lower snake for their configs, and individuals will be copying those. If we don't go lower snake we'll end up seeing configs in the wild sharing both, which nobody wants I'm sure.

Lastly, users (like me) who intend to use SpeedTracker on CloudCannon will benefit from automatic spacing and pluralization of metadata when using lower snake case: https://docs.cloudcannon.com/editing/front-matter/

@eduardoboucas
Copy link
Member

That's fine with me. As for changing wptUrl accordingly, I wrote some thoughts on #59.

@ghost
Copy link
Author

ghost commented Apr 25, 2017

Still looking for a merge on this. Please let me know if there is any actionable feedback.

@eduardoboucas
Copy link
Member

Sorry for the delay, been distracted with other things. I'll merge and release later today.

@eduardoboucas eduardoboucas merged commit 5c5d9ba into speedtracker:master May 2, 2017
@eduardoboucas
Copy link
Member

Shamefully late, merged!

@ghost ghost deleted the analytics branch May 3, 2017 04:18
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.

Integrate Tracking with Google Analytics
1 participant